From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752732AbbBRHe7 (ORCPT ); Wed, 18 Feb 2015 02:34:59 -0500 Received: from mail-wi0-f169.google.com ([209.85.212.169]:45721 "EHLO mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751872AbbBRHe5 (ORCPT ); Wed, 18 Feb 2015 02:34:57 -0500 Date: Wed, 18 Feb 2015 10:34:55 +0300 From: Alexey Dobriyan To: Jan Kara Cc: akpm@linux-foundation.org, viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, swhiteho@redhat.com, cluster-devel@redhat.com Subject: Re: [PATCH] fs: record task name which froze superblock Message-ID: <20150218073455.GA1752@p183.telecom.by> References: <20150214185524.GA16579@p183.telecom.by> <20150216093852.GB4749@quack.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150216093852.GB4749@quack.suse.cz> User-Agent: Mutt/1.5.22 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > 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. > > --- 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. > Also you seem to be missing freezing / thawing in freeze/thaw_bdev() > functions. You are correct. Resending patch (blockdev freezing tested with XFS). > > --- 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? It will pull sched.h, dunno if we care about headers anymore. Alexey