Messages in this thread | | | Date | Wed, 18 Feb 2015 10:18:12 +0000 | From | Steven Whitehouse <> | Subject | Re: [PATCH] fs: record task name which froze superblock |
| |
Hi,
On 18/02/15 09:13, Jan Kara wrote: > On Wed 18-02-15 10:34:55, Alexey Dobriyan wrote: >> On Mon, Feb 16, 2015 at 10:38:52AM +0100, Jan Kara wrote: >>> On Sat 14-02-15 21:55:24, Alexey Dobriyan wrote: >>>> Freezing and thawing are separate system calls, task which is supposed >>>> to thaw filesystem/superblock can disappear due to crash or not thaw >>>> due to a bug. Record at least task name (we can't take task_struct >>>> reference) to make support engineer's life easier. >>>> >>>> Hopefully 16 bytes per superblock isn't much. >>>> >>>> P.S.: Cc'ing GFS2 people just in case they want to correct >>>> my understanding of GFS2 having async freeze code. >>>> >>>> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> >>> Hum, and when do you show the task name? Or do you expect that customer >>> takes a crashdump and support just finds it in memory? >> Yeah, having at least something in crashdump is fine. > OK, then comment about this at freeze_comm[] definition so that it's > clear it isn't just set-but-never-read field. > >>>> --- a/fs/ioctl.c >>>> +++ b/fs/ioctl.c >>>> @@ -518,6 +518,7 @@ static int ioctl_fioasync(unsigned int fd, struct file *filp, >>>> static int ioctl_fsfreeze(struct file *filp) >>>> { >>>> struct super_block *sb = file_inode(filp)->i_sb; >>>> + int rv; >>>> >>>> if (!capable(CAP_SYS_ADMIN)) >>>> return -EPERM; >>>> @@ -527,22 +528,31 @@ static int ioctl_fsfreeze(struct file *filp) >>>> return -EOPNOTSUPP; >>>> >>>> /* Freeze */ >>>> - if (sb->s_op->freeze_super) >>>> - return sb->s_op->freeze_super(sb); >>>> - return freeze_super(sb); >>>> + if (sb->s_op->freeze_super) { >>>> + rv = sb->s_op->freeze_super(sb); >>>> + if (rv == 0) >>>> + get_task_comm(sb->s_writers.freeze_comm, current); >>>> + } else >>>> + rv = freeze_super(sb); >>>> + return rv; >>> Why don't you just set the name in ioctl_fsfreeze() in both cases? >> There are users of freeze_super() in GFS2 unless I'm misreading code. > Yes, there are. The call in fs/gfs2/glops.c is in a call path from > ->freeze_super() handler for GFS2 so that one is handled in > ioctl_fsfreeze() anyway. The call in fs/gfs2/sys.c is a way to freeze > filesystem via sysfs (dunno why GFS2 has to invent its own thing and ioctl > isn't enough). Steven? So having the logic in ioctl_fsfreeze(), > freeze_bdev() and freeze_store() in gfs2 seems to be enough. > The sysfs freeze thing is historical and strongly deprecated - I hope that we may be able to remove it one day,
Steve.
| |