2011-03-03 07:32:21

by Ken Sumrall

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

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 <[email protected]>
---
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


2011-03-03 08:19:07

by Ingo Molnar

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


* Ken Sumrall <[email protected]> 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 <[email protected]>
> ---
> 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

2011-03-03 08:46:54

by Dave Young

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

On Thu, Mar 3, 2011 at 3:31 PM, Ken Sumrall <[email protected]> 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 <[email protected]>
> ---
>  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

2011-03-03 08:49:59

by Dave Young

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

On Thu, Mar 3, 2011 at 4:46 PM, Dave Young <[email protected]> wrote:
> On Thu, Mar 3, 2011 at 3:31 PM, Ken Sumrall <[email protected]> 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 <[email protected]>
>> ---
>>  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

2011-03-03 14:01:01

by Christoph Hellwig

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

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.

2011-03-03 15:57:40

by Phil Carmody

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

On 02/03/11 23:31 -0800, ext 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.
>
> Signed-off-by: Ken Sumrall <[email protected]>
> ---
...
> 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;

What happened to fall through cases?

switch (cmd) {
+ case LINUX_REBOOT_CMD_RMNT_RESTART:
+ emergency_remount_synchronous();
case LINUX_REBOOT_CMD_RESTART:
kernel_restart(NULL);
break;

Ditto for the other related hunks.

Then again, I agree with the other responders, in particular those about fixing
problems with init in init itself.

Phil

2011-03-03 17:46:10

by Linus Torvalds

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

On Wed, Mar 2, 2011 at 11:31 PM, Ken Sumrall <[email protected]> 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

2011-03-03 18:18:24

by Linus Torvalds

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

On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
<[email protected]> 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

2011-03-03 18:28:05

by Mark Lord

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

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.

2011-03-03 18:29:56

by Mark Lord

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

On 11-03-03 01:17 PM, Linus Torvalds wrote:
> On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
> <[email protected]> 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

2011-03-03 23:23:25

by Ken Sumrall

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

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 <[email protected]> wrote:
> On 11-03-03 01:17 PM, Linus Torvalds wrote:
>> On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
>> <[email protected]> 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
>
>

2011-03-03 23:37:08

by Linus Torvalds

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

On Thu, Mar 3, 2011 at 10:28 AM, Mark Lord <[email protected]> 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

2011-03-04 02:00:47

by Ken Sumrall

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

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
<[email protected]> wrote:
> On Thu, Mar 3, 2011 at 10:28 AM, Mark Lord <[email protected]> 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
>

2011-03-04 02:34:34

by Linus Torvalds

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

On Thu, Mar 3, 2011 at 6:00 PM, Ken Sumrall <[email protected]> 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

2011-03-04 02:48:16

by Mark Lord

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

On 11-03-03 09:33 PM, Linus Torvalds wrote:
> On Thu, Mar 3, 2011 at 6:00 PM, Ken Sumrall <[email protected]> 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

2011-03-04 02:55:13

by Scott James Remnant

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

On Thu, Mar 3, 2011 at 6:48 PM, Mark Lord <[email protected]> wrote:
> On 11-03-03 09:33 PM, Linus Torvalds wrote:
>> On Thu, Mar 3, 2011 at 6:00 PM, Ken Sumrall <[email protected]> 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

2011-03-04 06:56:54

by Artem Bityutskiy

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

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 (Артём Битюцкий)

2011-03-05 02:45:25

by Mark Lord

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

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

2011-03-07 11:00:25

by Dimitris Papastamos

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

On Thu, Mar 03, 2011 at 10:17:32AM -0800, Linus Torvalds wrote:
> On Thu, Mar 3, 2011 at 9:45 AM, Linus Torvalds
> <[email protected]> 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