From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932324AbbBPJi5 (ORCPT ); Mon, 16 Feb 2015 04:38:57 -0500 Received: from cantor2.suse.de ([195.135.220.15]:45313 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754066AbbBPJi4 (ORCPT ); Mon, 16 Feb 2015 04:38:56 -0500 Date: Mon, 16 Feb 2015 10:38:52 +0100 From: Jan Kara To: Alexey Dobriyan Cc: akpm@linux-foundation.org, viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, jack@suse.cz, linux-fsdevel@vger.kernel.org, swhiteho@redhat.com, cluster-devel@redhat.com Subject: Re: [PATCH] fs: record task name which froze superblock Message-ID: <20150216093852.GB4749@quack.suse.cz> References: <20150214185524.GA16579@p183.telecom.by> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150214185524.GA16579@p183.telecom.by> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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? > --- > > fs/ioctl.c | 22 ++++++++++++++++------ > fs/super.c | 2 ++ > include/linux/fs.h | 2 ++ > 3 files changed, 20 insertions(+), 6 deletions(-) > > --- 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? Also you seem to be missing freezing / thawing in freeze/thaw_bdev() functions. > } > > static int ioctl_fsthaw(struct file *filp) > { > struct super_block *sb = file_inode(filp)->i_sb; > + int rv; > > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > > /* Thaw */ > - if (sb->s_op->thaw_super) > - return sb->s_op->thaw_super(sb); > - return thaw_super(sb); > + if (sb->s_op->thaw_super) { > + rv = sb->s_op->thaw_super(sb); > + if (rv == 0) > + memset(sb->s_writers.freeze_comm, 0, TASK_COMM_LEN); > + } else > + rv = thaw_super(sb); > + return rv; > } > > /* > --- a/fs/super.c > +++ b/fs/super.c > @@ -1355,6 +1355,7 @@ int freeze_super(struct super_block *sb) > * sees write activity when frozen is set to SB_FREEZE_COMPLETE. > */ > sb->s_writers.frozen = SB_FREEZE_COMPLETE; > + get_task_comm(sb->s_writers.freeze_comm, current); > up_write(&sb->s_umount); > return 0; > } > @@ -1391,6 +1392,7 @@ int thaw_super(struct super_block *sb) > > out: > sb->s_writers.frozen = SB_UNFROZEN; > + memset(sb->s_writers.freeze_comm, 0, TASK_COMM_LEN); > smp_wmb(); > wake_up(&sb->s_writers.wait_unfrozen); > deactivate_locked_super(sb); > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1221,6 +1221,8 @@ struct sb_writers { > int frozen; /* Is sb frozen? */ > wait_queue_head_t wait_unfrozen; /* queue for waiting for > sb to be thawed */ > + /* who froze superblock */ > + char freeze_comm[16]; Here should be TASK_COMM_LEN, shouldn't it? Honza -- Jan Kara SUSE Labs, CR