[PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Ken Sumrall]

Add 4 new commands to the reboot system call, that do the same thing as the
RESTART, HALT, POWER_OFF, and RESTART2 commands, but also remount writable
filesystems as read-only just before doing what the command normally does.
Now that Android is using EXT4, and since we don't have a standard init
setup to unmount filesystems before rebooting, this allows the system to
reboot with clean filesystems, and also improves boot time as the journal
does not need to be replayed when mounting the filesystem.

Signed-off-by: Ken Sumrall <ksumrall@android.com>
---
 fs/super.c             |    9 +++++++++
 include/linux/fs.h     |    1 +
 include/linux/reboot.h |    4 ++++
 kernel/sys.c           |   12 ++++++++++++
 4 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 8819e3a..3f39a16 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -638,6 +638,15 @@ void emergency_remount(void)
 	}
 }
 
+void emergency_remount_synchronous(void)
+{
+	struct work_struct *work;
+
+	work = kmalloc(sizeof(*work), GFP_ATOMIC);
+	if (work)
+		do_emergency_remount(work);
+}
+
 /*
  * Unnamed block devices are dummy devices used by virtual
  * filesystems which don't use real block-devices.  -- jrs
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 63d069b..e48ef0d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2110,6 +2110,7 @@ extern int generic_write_sync(struct file *file, loff_t pos, loff_t count);
 extern void sync_supers(void);
 extern void emergency_sync(void);
 extern void emergency_remount(void);
+extern void emergency_remount_synchronous(void);
 #ifdef CONFIG_BLOCK
 extern sector_t bmap(struct inode *, sector_t);
 #endif
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 3005d5a..24b185d 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -26,11 +26,15 @@
  */
 
 #define	LINUX_REBOOT_CMD_RESTART	0x01234567
+#define	LINUX_REBOOT_CMD_RMNT_RESTART	0x12345670
 #define	LINUX_REBOOT_CMD_HALT		0xCDEF0123
+#define	LINUX_REBOOT_CMD_RMNT_HALT	0xDEF0123C
 #define	LINUX_REBOOT_CMD_CAD_ON		0x89ABCDEF
 #define	LINUX_REBOOT_CMD_CAD_OFF	0x00000000
 #define	LINUX_REBOOT_CMD_POWER_OFF	0x4321FEDC
+#define	LINUX_REBOOT_CMD_RMNT_POWER_OFF	0x321FEDC4
 #define	LINUX_REBOOT_CMD_RESTART2	0xA1B2C3D4
+#define	LINUX_REBOOT_CMD_RMNT_RESTART2	0x1B2C3D4A
 #define	LINUX_REBOOT_CMD_SW_SUSPEND	0xD000FCE2
 #define	LINUX_REBOOT_CMD_KEXEC		0x45584543
 
diff --git a/kernel/sys.c b/kernel/sys.c
index 7f5a0cd..3f474e6 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -392,6 +392,9 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
 	mutex_lock(&reboot_mutex);
 	switch (cmd) {
 	case LINUX_REBOOT_CMD_RESTART:
+	case LINUX_REBOOT_CMD_RMNT_RESTART:
+		if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART)
+			emergency_remount_synchronous();
 		kernel_restart(NULL);
 		break;
 
@@ -404,22 +407,31 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
 		break;
 
 	case LINUX_REBOOT_CMD_HALT:
+	case LINUX_REBOOT_CMD_RMNT_HALT:
+		if (cmd == LINUX_REBOOT_CMD_RMNT_HALT)
+			emergency_remount_synchronous();
 		kernel_halt();
 		do_exit(0);
 		panic("cannot halt");
 
 	case LINUX_REBOOT_CMD_POWER_OFF:
+	case LINUX_REBOOT_CMD_RMNT_POWER_OFF:
+		if (cmd == LINUX_REBOOT_CMD_RMNT_POWER_OFF)
+			emergency_remount_synchronous();
 		kernel_power_off();
 		do_exit(0);
 		break;
 
 	case LINUX_REBOOT_CMD_RESTART2:
+	case LINUX_REBOOT_CMD_RMNT_RESTART2:
 		if (strncpy_from_user(&buffer[0], arg, sizeof(buffer) - 1) < 0) {
 			ret = -EFAULT;
 			break;
 		}
 		buffer[sizeof(buffer) - 1] = '\0';
 
+		if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART2)
+			emergency_remount_synchronous();
 		kernel_restart(buffer);
 		break;
 
-- 
1.7.3.1

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Ingo Molnar]

* Ken Sumrall <ksumrall@android.com> wrote:

> Add 4 new commands to the reboot system call, that do the same thing as the
> RESTART, HALT, POWER_OFF, and RESTART2 commands, but also remount writable
> filesystems as read-only just before doing what the command normally does.
> Now that Android is using EXT4, and since we don't have a standard init
> setup to unmount filesystems before rebooting, this allows the system to
> reboot with clean filesystems, and also improves boot time as the journal
> does not need to be replayed when mounting the filesystem.
> 
> Signed-off-by: Ken Sumrall <ksumrall@android.com>
> ---
>  fs/super.c             |    9 +++++++++
>  include/linux/fs.h     |    1 +
>  include/linux/reboot.h |    4 ++++
>  kernel/sys.c           |   12 ++++++++++++
>  4 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 8819e3a..3f39a16 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -638,6 +638,15 @@ void emergency_remount(void)
>  	}
>  }
>  
> +void emergency_remount_synchronous(void)
> +{
> +	struct work_struct *work;
> +
> +	work = kmalloc(sizeof(*work), GFP_ATOMIC);
> +	if (work)
> +		do_emergency_remount(work);
> +}
> +
>  /*
>   * Unnamed block devices are dummy devices used by virtual
>   * filesystems which don't use real block-devices.  -- jrs
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 63d069b..e48ef0d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2110,6 +2110,7 @@ extern int generic_write_sync(struct file *file, loff_t pos, loff_t count);
>  extern void sync_supers(void);
>  extern void emergency_sync(void);
>  extern void emergency_remount(void);
> +extern void emergency_remount_synchronous(void);
>  #ifdef CONFIG_BLOCK
>  extern sector_t bmap(struct inode *, sector_t);
>  #endif
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 3005d5a..24b185d 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -26,11 +26,15 @@
>   */
>  
>  #define	LINUX_REBOOT_CMD_RESTART	0x01234567
> +#define	LINUX_REBOOT_CMD_RMNT_RESTART	0x12345670
>  #define	LINUX_REBOOT_CMD_HALT		0xCDEF0123
> +#define	LINUX_REBOOT_CMD_RMNT_HALT	0xDEF0123C
>  #define	LINUX_REBOOT_CMD_CAD_ON		0x89ABCDEF
>  #define	LINUX_REBOOT_CMD_CAD_OFF	0x00000000
>  #define	LINUX_REBOOT_CMD_POWER_OFF	0x4321FEDC
> +#define	LINUX_REBOOT_CMD_RMNT_POWER_OFF	0x321FEDC4
>  #define	LINUX_REBOOT_CMD_RESTART2	0xA1B2C3D4
> +#define	LINUX_REBOOT_CMD_RMNT_RESTART2	0x1B2C3D4A
>  #define	LINUX_REBOOT_CMD_SW_SUSPEND	0xD000FCE2
>  #define	LINUX_REBOOT_CMD_KEXEC		0x45584543
>  
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 7f5a0cd..3f474e6 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -392,6 +392,9 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
>  	mutex_lock(&reboot_mutex);
>  	switch (cmd) {
>  	case LINUX_REBOOT_CMD_RESTART:
> +	case LINUX_REBOOT_CMD_RMNT_RESTART:
> +		if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART)
> +			emergency_remount_synchronous();
>  		kernel_restart(NULL);
>  		break;
>  
> @@ -404,22 +407,31 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
>  		break;
>  
>  	case LINUX_REBOOT_CMD_HALT:
> +	case LINUX_REBOOT_CMD_RMNT_HALT:
> +		if (cmd == LINUX_REBOOT_CMD_RMNT_HALT)
> +			emergency_remount_synchronous();
>  		kernel_halt();
>  		do_exit(0);
>  		panic("cannot halt");
>  
>  	case LINUX_REBOOT_CMD_POWER_OFF:
> +	case LINUX_REBOOT_CMD_RMNT_POWER_OFF:
> +		if (cmd == LINUX_REBOOT_CMD_RMNT_POWER_OFF)
> +			emergency_remount_synchronous();
>  		kernel_power_off();
>  		do_exit(0);
>  		break;
>  
>  	case LINUX_REBOOT_CMD_RESTART2:
> +	case LINUX_REBOOT_CMD_RMNT_RESTART2:
>  		if (strncpy_from_user(&buffer[0], arg, sizeof(buffer) - 1) < 0) {
>  			ret = -EFAULT;
>  			break;
>  		}
>  		buffer[sizeof(buffer) - 1] = '\0';
>  
> +		if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART2)
> +			emergency_remount_synchronous();
>  		kernel_restart(buffer);
>  		break;

Wouldnt it be *much* simpler to add it as a magic1 variant:

 #define LINUX_REBOOT_UMOUNT_RO   0xfee1deaf

Used as a 'also remount ro please' flag?

That way the whole patch would literally be 2 lines:

	if (magic1 == LINUX_REBOOT_UMOUNT_RO)
		emergency_remount_synchronous();

Note that this also has the advantage that both kexec and suspend reboots could be 
done with emergency umounts, if so desired - and any future reboot variant would be 
supported as well. Your patch left out those other reboot methods.

Thanks,

	Ingo

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Dave Young]

On Thu, Mar 3, 2011 at 3:31 PM, Ken Sumrall <ksumrall@android.com> wrote:
> Add 4 new commands to the reboot system call, that do the same thing as the
> RESTART, HALT, POWER_OFF, and RESTART2 commands, but also remount writable
> filesystems as read-only just before doing what the command normally does.
> Now that Android is using EXT4, and since we don't have a standard init
> setup to unmount filesystems before rebooting, this allows the system to
> reboot with clean filesystems, and also improves boot time as the journal
> does not need to be replayed when mounting the filesystem.
>
> Signed-off-by: Ken Sumrall <ksumrall@android.com>
> ---
>  fs/super.c             |    9 +++++++++
>  include/linux/fs.h     |    1 +
>  include/linux/reboot.h |    4 ++++
>  kernel/sys.c           |   12 ++++++++++++
>  4 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 8819e3a..3f39a16 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -638,6 +638,15 @@ void emergency_remount(void)
>        }
>  }
>
> +void emergency_remount_synchronous(void)
> +{
> +       struct work_struct *work;
> +
> +       work = kmalloc(sizeof(*work), GFP_ATOMIC);
> +       if (work)
> +               do_emergency_remount(work);

work is not needed here? just call do_mergency_remount()

> +}
> +
>  /*
>  * Unnamed block devices are dummy devices used by virtual
>  * filesystems which don't use real block-devices.  -- jrs
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 63d069b..e48ef0d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2110,6 +2110,7 @@ extern int generic_write_sync(struct file *file, loff_t pos, loff_t count);
>  extern void sync_supers(void);
>  extern void emergency_sync(void);
>  extern void emergency_remount(void);
> +extern void emergency_remount_synchronous(void);
>  #ifdef CONFIG_BLOCK
>  extern sector_t bmap(struct inode *, sector_t);
>  #endif
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 3005d5a..24b185d 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -26,11 +26,15 @@
>  */
>
>  #define        LINUX_REBOOT_CMD_RESTART        0x01234567
> +#define        LINUX_REBOOT_CMD_RMNT_RESTART   0x12345670
>  #define        LINUX_REBOOT_CMD_HALT           0xCDEF0123
> +#define        LINUX_REBOOT_CMD_RMNT_HALT      0xDEF0123C
>  #define        LINUX_REBOOT_CMD_CAD_ON         0x89ABCDEF
>  #define        LINUX_REBOOT_CMD_CAD_OFF        0x00000000
>  #define        LINUX_REBOOT_CMD_POWER_OFF      0x4321FEDC
> +#define        LINUX_REBOOT_CMD_RMNT_POWER_OFF 0x321FEDC4
>  #define        LINUX_REBOOT_CMD_RESTART2       0xA1B2C3D4
> +#define        LINUX_REBOOT_CMD_RMNT_RESTART2  0x1B2C3D4A
>  #define        LINUX_REBOOT_CMD_SW_SUSPEND     0xD000FCE2
>  #define        LINUX_REBOOT_CMD_KEXEC          0x45584543
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 7f5a0cd..3f474e6 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -392,6 +392,9 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
>        mutex_lock(&reboot_mutex);
>        switch (cmd) {
>        case LINUX_REBOOT_CMD_RESTART:
> +       case LINUX_REBOOT_CMD_RMNT_RESTART:
> +               if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART)
> +                       emergency_remount_synchronous();
>                kernel_restart(NULL);
>                break;
>
> @@ -404,22 +407,31 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
>                break;
>
>        case LINUX_REBOOT_CMD_HALT:
> +       case LINUX_REBOOT_CMD_RMNT_HALT:
> +               if (cmd == LINUX_REBOOT_CMD_RMNT_HALT)
> +                       emergency_remount_synchronous();
>                kernel_halt();
>                do_exit(0);
>                panic("cannot halt");
>
>        case LINUX_REBOOT_CMD_POWER_OFF:
> +       case LINUX_REBOOT_CMD_RMNT_POWER_OFF:
> +               if (cmd == LINUX_REBOOT_CMD_RMNT_POWER_OFF)
> +                       emergency_remount_synchronous();
>                kernel_power_off();
>                do_exit(0);
>                break;
>
>        case LINUX_REBOOT_CMD_RESTART2:
> +       case LINUX_REBOOT_CMD_RMNT_RESTART2:
>                if (strncpy_from_user(&buffer[0], arg, sizeof(buffer) - 1) < 0) {
>                        ret = -EFAULT;
>                        break;
>                }
>                buffer[sizeof(buffer) - 1] = '\0';
>
> +               if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART2)
> +                       emergency_remount_synchronous();
>                kernel_restart(buffer);
>                break;
>
> --
> 1.7.3.1
>
>



-- 
Regards
dave

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Dave Young]

On Thu, Mar 3, 2011 at 4:46 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Thu, Mar 3, 2011 at 3:31 PM, Ken Sumrall <ksumrall@android.com> wrote:
>> Add 4 new commands to the reboot system call, that do the same thing as the
>> RESTART, HALT, POWER_OFF, and RESTART2 commands, but also remount writable
>> filesystems as read-only just before doing what the command normally does.
>> Now that Android is using EXT4, and since we don't have a standard init
>> setup to unmount filesystems before rebooting, this allows the system to
>> reboot with clean filesystems, and also improves boot time as the journal
>> does not need to be replayed when mounting the filesystem.
>>
>> Signed-off-by: Ken Sumrall <ksumrall@android.com>
>> ---
>>  fs/super.c             |    9 +++++++++
>>  include/linux/fs.h     |    1 +
>>  include/linux/reboot.h |    4 ++++
>>  kernel/sys.c           |   12 ++++++++++++
>>  4 files changed, 26 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/super.c b/fs/super.c
>> index 8819e3a..3f39a16 100644
>> --- a/fs/super.c
>> +++ b/fs/super.c
>> @@ -638,6 +638,15 @@ void emergency_remount(void)
>>        }
>>  }
>>
>> +void emergency_remount_synchronous(void)
>> +{
>> +       struct work_struct *work;
>> +
>> +       work = kmalloc(sizeof(*work), GFP_ATOMIC);
>> +       if (work)
>> +               do_emergency_remount(work);
>
> work is not needed here? just call do_mergency_remount()

fix above comment: still need a null pointer

>
>> +}
>> +
>>  /*
>>  * Unnamed block devices are dummy devices used by virtual
>>  * filesystems which don't use real block-devices.  -- jrs
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 63d069b..e48ef0d 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2110,6 +2110,7 @@ extern int generic_write_sync(struct file *file, loff_t pos, loff_t count);
>>  extern void sync_supers(void);
>>  extern void emergency_sync(void);
>>  extern void emergency_remount(void);
>> +extern void emergency_remount_synchronous(void);
>>  #ifdef CONFIG_BLOCK
>>  extern sector_t bmap(struct inode *, sector_t);
>>  #endif
>> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
>> index 3005d5a..24b185d 100644
>> --- a/include/linux/reboot.h
>> +++ b/include/linux/reboot.h
>> @@ -26,11 +26,15 @@
>>  */
>>
>>  #define        LINUX_REBOOT_CMD_RESTART        0x01234567
>> +#define        LINUX_REBOOT_CMD_RMNT_RESTART   0x12345670
>>  #define        LINUX_REBOOT_CMD_HALT           0xCDEF0123
>> +#define        LINUX_REBOOT_CMD_RMNT_HALT      0xDEF0123C
>>  #define        LINUX_REBOOT_CMD_CAD_ON         0x89ABCDEF
>>  #define        LINUX_REBOOT_CMD_CAD_OFF        0x00000000
>>  #define        LINUX_REBOOT_CMD_POWER_OFF      0x4321FEDC
>> +#define        LINUX_REBOOT_CMD_RMNT_POWER_OFF 0x321FEDC4
>>  #define        LINUX_REBOOT_CMD_RESTART2       0xA1B2C3D4
>> +#define        LINUX_REBOOT_CMD_RMNT_RESTART2  0x1B2C3D4A
>>  #define        LINUX_REBOOT_CMD_SW_SUSPEND     0xD000FCE2
>>  #define        LINUX_REBOOT_CMD_KEXEC          0x45584543
>>
>> diff --git a/kernel/sys.c b/kernel/sys.c
>> index 7f5a0cd..3f474e6 100644
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
>> @@ -392,6 +392,9 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
>>        mutex_lock(&reboot_mutex);
>>        switch (cmd) {
>>        case LINUX_REBOOT_CMD_RESTART:
>> +       case LINUX_REBOOT_CMD_RMNT_RESTART:
>> +               if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART)
>> +                       emergency_remount_synchronous();
>>                kernel_restart(NULL);
>>                break;
>>
>> @@ -404,22 +407,31 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
>>                break;
>>
>>        case LINUX_REBOOT_CMD_HALT:
>> +       case LINUX_REBOOT_CMD_RMNT_HALT:
>> +               if (cmd == LINUX_REBOOT_CMD_RMNT_HALT)
>> +                       emergency_remount_synchronous();
>>                kernel_halt();
>>                do_exit(0);
>>                panic("cannot halt");
>>
>>        case LINUX_REBOOT_CMD_POWER_OFF:
>> +       case LINUX_REBOOT_CMD_RMNT_POWER_OFF:
>> +               if (cmd == LINUX_REBOOT_CMD_RMNT_POWER_OFF)
>> +                       emergency_remount_synchronous();
>>                kernel_power_off();
>>                do_exit(0);
>>                break;
>>
>>        case LINUX_REBOOT_CMD_RESTART2:
>> +       case LINUX_REBOOT_CMD_RMNT_RESTART2:
>>                if (strncpy_from_user(&buffer[0], arg, sizeof(buffer) - 1) < 0) {
>>                        ret = -EFAULT;
>>                        break;
>>                }
>>                buffer[sizeof(buffer) - 1] = '\0';
>>
>> +               if (cmd == LINUX_REBOOT_CMD_RMNT_RESTART2)
>> +                       emergency_remount_synchronous();
>>                kernel_restart(buffer);
>>                break;
>>
>> --
>> 1.7.3.1
>>
>>
>
>
>
> --
> Regards
> dave
>



-- 
Regards
dave

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Christoph Hellwig]

On Wed, Mar 02, 2011 at 11:31:22PM -0800, Ken Sumrall wrote:
> Add 4 new commands to the reboot system call, that do the same thing as the
> RESTART, HALT, POWER_OFF, and RESTART2 commands, but also remount writable
> filesystems as read-only just before doing what the command normally does.
> Now that Android is using EXT4, and since we don't have a standard init
> setup to unmount filesystems before rebooting, this allows the system to
> reboot with clean filesystems, and also improves boot time as the journal
> does not need to be replayed when mounting the filesystem.

Just fix your init.  Or better switch from a piece of shit like Android
userspace to something sane for your phone.

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Linus Torvalds]

On Wed, Mar 2, 2011 at 11:31 PM, Ken Sumrall <ksumrall@android.com> wrote:
>
> Add 4 new commands to the reboot system call, that do the same thing as the
> RESTART, HALT, POWER_OFF, and RESTART2 commands, but also remount writable
> filesystems as read-only just before doing what the command normally does.

This makes no sense.

If you can change whatever user-land process that does the reboot
system call (and clearly you can, since you're adding new commands and
using those), then why the heck don't you just do the remount-ro from
that same user land?

How many mounted filesystems do you have that it's so hard to keep track of?

                      Linus

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Linus Torvalds]

On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> How many mounted filesystems do you have that it's so hard to keep track of?

Btw, /proc/mounts will track them for you even if you don't have a
'mount' binary that does.

Parsing that is pretty trivial. If you have spaces or special
characters in your pathnames (you may control the mount paths, you may
not - I have no idea), you'll need to be able to handle the escape
format (\oct). But other than that, it's literally just

 - read all of /proc/mounts into a buffer

 - for each line, split by space, and you'll have the directory name
right there in the second field

 - do the unescaping ("\oct" -> character) if needed. It's good
practice. Test it.

 - just do a read-only remount on it.

All done. No kernel changes necessary. It just works.

                            Linus

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Mark Lord]

On 11-03-03 12:45 PM, Linus Torvalds wrote:
>
> If you can change whatever user-land process that does the reboot
> system call (and clearly you can, since you're adding new commands and
> using those), then why the heck don't you just do the remount-ro from
> that same user land?

Is there a system call for emergency_remount_*() ?
I've been writing to /proc/proc/sysrq-trigger to accomplish this.

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Mark Lord]

On 11-03-03 01:17 PM, Linus Torvalds wrote:
> On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> How many mounted filesystems do you have that it's so hard to keep track of?
> 
> Btw, /proc/mounts will track them for you even if you don't have a
> 'mount' binary that does.
> 
> Parsing that is pretty trivial. If you have spaces or special
> characters in your pathnames (you may control the mount paths, you may
> not - I have no idea), you'll need to be able to handle the escape
> format (\oct). But other than that, it's literally just
> 
>  - read all of /proc/mounts into a buffer
> 
>  - for each line, split by space, and you'll have the directory name
> right there in the second field
> 
>  - do the unescaping ("\oct" -> character) if needed. It's good
> practice. Test it.
> 
>  - just do a read-only remount on it.
> 
> All done. No kernel changes necessary. It just works.


This might be much less complex:

   #!/bin/sh
   echo s > /proc/sysrq-trigger
   echo u > /proc/sysrq-trigger

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Ken Sumrall]

Thanks for the feedback.  I suspected the kernel community might not
like this change.  The suggestions by Linus seem worth trying out.

___
Ken


On Thu, Mar 3, 2011 at 10:29 AM, Mark Lord <kernel@teksavvy.com> wrote:
> On 11-03-03 01:17 PM, Linus Torvalds wrote:
>> On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>>
>>> How many mounted filesystems do you have that it's so hard to keep track of?
>>
>> Btw, /proc/mounts will track them for you even if you don't have a
>> 'mount' binary that does.
>>
>> Parsing that is pretty trivial. If you have spaces or special
>> characters in your pathnames (you may control the mount paths, you may
>> not - I have no idea), you'll need to be able to handle the escape
>> format (\oct). But other than that, it's literally just
>>
>>  - read all of /proc/mounts into a buffer
>>
>>  - for each line, split by space, and you'll have the directory name
>> right there in the second field
>>
>>  - do the unescaping ("\oct" -> character) if needed. It's good
>> practice. Test it.
>>
>>  - just do a read-only remount on it.
>>
>> All done. No kernel changes necessary. It just works.
>
>
> This might be much less complex:
>
>   #!/bin/sh
>   echo s > /proc/sysrq-trigger
>   echo u > /proc/sysrq-trigger
>
>

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Linus Torvalds]

On Thu, Mar 3, 2011 at 10:28 AM, Mark Lord <kernel@teksavvy.com> wrote:
> On 11-03-03 12:45 PM, Linus Torvalds wrote:
>>
>> If you can change whatever user-land process that does the reboot
>> system call (and clearly you can, since you're adding new commands and
>> using those), then why the heck don't you just do the remount-ro from
>> that same user land?
>
> Is there a system call for emergency_remount_*() ?
> I've been writing to /proc/proc/sysrq-trigger to accomplish this.

So I don't know what this has to do with "emergency_remount()" - we're
talking about a regular controlled shutdown/reset. The fact that the
patch used the emergency_remount() code seems to be purely an
implementation issue, nothing more.

And while sysrq-trigger certainly works (when it's enabled, but that's
true of /proc too, of course), I do think it's a rather odd way of
solving the problem, when the simple "just remount read-only" is what
the code actually _wants_ to do.

But you're certainly right that it takes less code to open
/proc/sysrq-trigger and writing a single byte to it than it does to do
the straightforward "let's just do the normal mount thing".

                             Linus

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Ken Sumrall]

Writing a single byte to /proc/sysrq-trigger is an asynchronous
operation, with no obvious way to be informed that it has completed
the remount.  I did actually try this at first, but the reboot
happened before the remount finished.  I could of course add a sleep
of a few seconds, but just how long to wait is not obvious, nor is it
a guarantee that the remount will always finish in the time chosen.
I'm heading down the path of reading /proc/mounts and remounting all
read-wirte filesystems backed by a block device as read-only.

___
Ken


On Thu, Mar 3, 2011 at 3:36 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Mar 3, 2011 at 10:28 AM, Mark Lord <kernel@teksavvy.com> wrote:
>> On 11-03-03 12:45 PM, Linus Torvalds wrote:
>>>
>>> If you can change whatever user-land process that does the reboot
>>> system call (and clearly you can, since you're adding new commands and
>>> using those), then why the heck don't you just do the remount-ro from
>>> that same user land?
>>
>> Is there a system call for emergency_remount_*() ?
>> I've been writing to /proc/proc/sysrq-trigger to accomplish this.
>
> So I don't know what this has to do with "emergency_remount()" - we're
> talking about a regular controlled shutdown/reset. The fact that the
> patch used the emergency_remount() code seems to be purely an
> implementation issue, nothing more.
>
> And while sysrq-trigger certainly works (when it's enabled, but that's
> true of /proc too, of course), I do think it's a rather odd way of
> solving the problem, when the simple "just remount read-only" is what
> the code actually _wants_ to do.
>
> But you're certainly right that it takes less code to open
> /proc/sysrq-trigger and writing a single byte to it than it does to do
> the straightforward "let's just do the normal mount thing".
>
>                             Linus
>

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Linus Torvalds]

On Thu, Mar 3, 2011 at 6:00 PM, Ken Sumrall <ksumrall@android.com> wrote:
>
> Writing a single byte to /proc/sysrq-trigger is an asynchronous
> operation, with no obvious way to be informed that it has completed
> the remount.

Right you are. That's something of a misfeature, but it comes from the
way sysrq works: obviously the "real" sysrq thing is about keyboard
input, so all the sysrq stuff has to be async.

The fact that that async nature then ends up also affecting the
/proc/sysrq-trigger case (which _could_ be synchronous) is a bit sad
in this case.

That said, I obviously think that just doing the read-only mount
yourself is the right thing to do regardless, and the sysrq thing
would have been just a cute/ugly hack if it had worked.

> I'm heading down the path of reading /proc/mounts and remounting all
> read-wirte filesystems backed by a block device as read-only.

So just as a practical matter: while it's quite possible that the nice
seq_printf() model of /proc/mounts means that it should work correctly
even if you read each line individually and then re-mount while
holding the file open, I would suggest that you read the whole file
into a buffer before you then start changing the mounts.

Otherwise, _if_ we ever were to actually move the filesystem on our
internal list of mounts when we re-mount it, you might otherwise end
up seeign the same filesystem multiple times (or the reverse - missing
some filesystem).

Basically, I'm saying that you should try to avoid changing mount
information in between read() calls to /proc/mounts. It might cause
confusion.

                                        Linus

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Mark Lord]

On 11-03-03 09:33 PM, Linus Torvalds wrote:
> On Thu, Mar 3, 2011 at 6:00 PM, Ken Sumrall <ksumrall@android.com> wrote:
>>
>> Writing a single byte to /proc/sysrq-trigger is an asynchronous
>> operation, with no obvious way to be informed that it has completed
>> the remount.
> 
> Right you are. That's something of a misfeature, but it comes from the
> way sysrq works: obviously the "real" sysrq thing is about keyboard
> input, so all the sysrq stuff has to be async.
> 
> The fact that that async nature then ends up also affecting the
> /proc/sysrq-trigger case (which _could_ be synchronous) is a bit sad
> in this case.

Agreed.

I have the echo s/u to sysrq-trigger (plus a 2-sec sleep ala MS-Win)
on my Ubuntu systems here, because their shutdown "sequence"
is racy and buggy, and frequently powers off the box with the
rootfs still mounted rw otherwise.

And don't get me started about the races on system startup
 -- "upstart" is an abomination, or at least the Ubuntu use of it is.

Cheers

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Scott James Remnant]

On Thu, Mar 3, 2011 at 6:48 PM, Mark Lord <kernel@teksavvy.com> wrote:
> On 11-03-03 09:33 PM, Linus Torvalds wrote:
>> On Thu, Mar 3, 2011 at 6:00 PM, Ken Sumrall <ksumrall@android.com> wrote:
>>>
>>> Writing a single byte to /proc/sysrq-trigger is an asynchronous
>>> operation, with no obvious way to be informed that it has completed
>>> the remount.
>>
>> Right you are. That's something of a misfeature, but it comes from the
>> way sysrq works: obviously the "real" sysrq thing is about keyboard
>> input, so all the sysrq stuff has to be async.
>>
>> The fact that that async nature then ends up also affecting the
>> /proc/sysrq-trigger case (which _could_ be synchronous) is a bit sad
>> in this case.
>
> Agreed.
>
> I have the echo s/u to sysrq-trigger (plus a 2-sec sleep ala MS-Win)
> on my Ubuntu systems here, because their shutdown "sequence"
> is racy and buggy, and frequently powers off the box with the
> rootfs still mounted rw otherwise.
>
> And don't get me started about the races on system startup
>  -- "upstart" is an abomination, or at least the Ubuntu use of it is.
>
Ubuntu doesn't use Upstart on shutdown, it uses the plain-old sysv init scripts.

Oddly enough I don't see any bugs filed by you about startup issues?
Now, don't get me wrong, I'm not saying Ubuntu is perfect here - but
I'm not aware of any races that are Upstart's fault. If there are,
please file bugs! How else am I to fix them if I don't know about
them?

Scott

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Artem Bityutskiy]

On Thu, 2011-03-03 at 15:36 -0800, Linus Torvalds wrote:
> But you're certainly right that it takes less code to open
> /proc/sysrq-trigger and writing a single byte to it than it does to do
> the straightforward "let's just do the normal mount thing".

/proc/sysrq-trigger is sometimes simply disabled for "security reasons".
I'm not sure about Android, but some systems I know do disable it. So I
think /proc/mounts is a cleaner approach.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Mark Lord]

On 11-03-03 09:55 PM, Scott James Remnant wrote:
>
> Oddly enough I don't see any bugs filed by you about startup issues?
> Now, don't get me wrong, I'm not saying Ubuntu is perfect here - but
> I'm not aware of any races that are Upstart's fault. If there are,
> please file bugs! How else am I to fix them if I don't know about
> them?


Oddly enough, I used to file bugs quite diligently against distros.
Not one of them was ever investigated or fixed.

Instead, I would get an automated email with each subsequent re-release
of the buggy software, inviting me to further waste my time by resubmitting
the bug report should it still be present.

So.. I simply don't bother now.

And upstart is used for shutdown of at least some services on Ubuntu.
I just wish they'd THINK about sequences.. like umounting NFS
before killing the network devices, or stopping mythbackend
before killing mysql etc..

If I could figure out how to fix their .conf files, I would.

Cheers

Re: [PATCH] Syscalls: reboot: Add options to the reboot syscall to remount filesystems ro

[Dimitris Papastamos]

On Thu, Mar 03, 2011 at 10:17:32AM -0800, Linus Torvalds wrote:
> On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > How many mounted filesystems do you have that it's so hard to keep track of?
> 
> Btw, /proc/mounts will track them for you even if you don't have a
> 'mount' binary that does.
> 
> Parsing that is pretty trivial. If you have spaces or special
> characters in your pathnames (you may control the mount paths, you may
> not - I have no idea), you'll need to be able to handle the escape
> format (\oct). But other than that, it's literally just
> 
>  - read all of /proc/mounts into a buffer
> 
>  - for each line, split by space, and you'll have the directory name
> right there in the second field
> 
>  - do the unescaping ("\oct" -> character) if needed. It's good
> practice. Test it.
> 
>  - just do a read-only remount on it.
> 
> All done. No kernel changes necessary. It just works.

That or using setmntent() etc.

Thanks,
Dimitris