From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mateusz Guzik Subject: Re: [PATCH V2 2/2] fs: print a message when freezing/unfreezing filesystems Date: Fri, 16 May 2014 00:34:40 +0200 Message-ID: <20140515223439.GA24089@mguzik.redhat.com> References: <1400018683-5565-2-git-send-email-mguzik@redhat.com> <20140514111449.GB5824@quack.suse.cz> <20140514112619.GA10637@mguzik.redhat.com> <20140514113945.GC5824@quack.suse.cz> <20140514220052.GD5421@dastard> <20140514223745.GF5421@dastard> <5373F0D6.4090600@redhat.com> <20140515104746.GF10637@mguzik.redhat.com> <20140515222135.GZ26353@dastard> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: =?utf-8?B?THVrw6HFoQ==?= Czerner , sandeen@redhat.com, Jan Kara , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Josef Bacik , Al Viro , Joe Perches To: Dave Chinner Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47978 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754830AbaEOWfD (ORCPT ); Thu, 15 May 2014 18:35:03 -0400 Content-Disposition: inline In-Reply-To: <20140515222135.GZ26353@dastard> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, May 16, 2014 at 08:21:35AM +1000, Dave Chinner wrote: > On Thu, May 15, 2014 at 12:47:48PM +0200, Mateusz Guzik wrote: > > On Thu, May 15, 2014 at 12:40:19PM +0200, Luk=C3=A1=C5=A1 Czerner w= rote: > > > On Wed, 14 May 2014, Eric Sandeen wrote: > > >=20 > > > > Date: Wed, 14 May 2014 17:40:22 -0500 > > > > From: Eric Sandeen > > > > Reply-To: sandeen@redhat.com > > > > To: Dave Chinner , Jan Kara > > > > Cc: Mateusz Guzik , linux-kernel@vger.kernel= =2Eorg, > > > > linux-fsdevel@vger.kernel.org, Josef Bacik , > > > > Al Viro , Joe Perches > > > > Subject: Re: [PATCH V2 2/2] fs: print a message when freezing/u= nfreezing > > > > filesystems > > > >=20 > > > > On 5/14/14, 5:37 PM, Dave Chinner wrote: > > > > > On Thu, May 15, 2014 at 08:00:52AM +1000, Dave Chinner wrote: > > > > >> On Wed, May 14, 2014 at 01:39:45PM +0200, Jan Kara wrote: > > > > >>> On Wed 14-05-14 13:26:21, Mateusz Guzik wrote: > > > > >>>> On Wed, May 14, 2014 at 01:14:49PM +0200, Jan Kara wrote: > > > > >>>>> On Wed 14-05-14 00:04:43, Mateusz Guzik wrote: > > > > >>>>>> This helps hang troubleshooting efforts when only dmesg = is available. > > > > >>>>>> > > > > >>>>>> While here remove code duplication with MS_RDONLY case a= nd fix a > > > > >>>>>> whitespace nit. > > > > >>>>> I'm somewhat undecided here I have to say. On one hand = I don't like > > > > >>>>> printing to kernel log when everything is fine and kernel= is operating > > > > >>>>> normally. On the other hand I've seen quite a few cases w= here people have > > > > >>>>> shot themselves in the foot with filesystem freezing so h= aving some trace > > > > >>>>> of this in the log doesn't seem like a completely bad thi= ng either. What do > > > > >>>>> other people think? > > > > >>>>> > > > > >>>> > > > > >>>> I would like to note that the kernel already prints messag= es when e.g. > > > > >>>> filesystems get mounted. > > > > >>> Yeah, that's a fair point. > > > > >> > > > > >> But filesystems choose to output that info, not the VFS. Whe= n you do > > > > >> a remount,ro there is no output in syslog, because filesyste= ms don't > > > > >> need to dump any output - the state change is reflected in > > > > >> /proc/self/mounts. IMO frozen should state should be communi= cated > > > > >> the same way so that it is silent when it just works, and th= e state > > > > >> can easily be determined when something goes wrong. > > > > >=20 > > > > > Say, like this: > > > > >=20 > > > > > $ grep /mnt/test /proc/mounts > > > > > /dev/vda /mnt/test xfs rw,relatime,attr2,inode64,noquota 0 0 > > > > > $ sudo xfs_freeze -f /mnt/test > > > > > $ grep /mnt/test /proc/mounts > > > > > /dev/vda /mnt/test xfs rw,frozen,relatime,attr2,inode64,noquo= ta 0 0 > > > > > $ sudo xfs_freeze -u /mnt/test > > > > > $ grep /mnt/test /proc/mounts > > > > > /dev/vda /mnt/test xfs rw,relatime,attr2,inode64,noquota 0 0 > > > > > $ > > > >=20 > > > > I'm not totally convinced that including a non-mount option in = what > > > > has always (?) been a list of mount options is a great idea. > > >=20 > > > I do not like it either. Mixing this together with other mount > > > options does not seem like a great idea, however we really need a > > > way to report this and I guess we can not just change the > > > /proc/self/mounts, or /proc/self/mountinfo format. > > >=20 > > > So what about crating a new file /proc/self/frozen with the list = of > > > frozen file systems in the same format what mounts, or mountinfo = has > > > ? > > >=20 > >=20 > > As the time goes on there will be more data to present about given > > mountpoint. So either mountinfo should be extendable to support thi= s > > (can't see why not) or new generic file with that property should b= e > > provided. I vote for the former. > >=20 > > IOW, a new column in mountinfo. For frozen filesystems it would con= tain > > 'frozen_by=3D[%s]:[%d]' (escaped comm, pid). >=20 > I really don't see that the process that froze the filesystem is > particularly useful - it many cases that process is long gone (e.g. > fsfreeze is being used to allow a HW array to take a snapshot). Just > the fact it is in the process of freezing (if stuck, stack trace in > sysrq-w should be present) or frozen (freezing process may be long > gone, and is mostly irrelevant because you're now tracking down why > a thaw hasn't happened)... >=20 There are deamons which perform freezing and unfreezing on their own. Thus storing the name along with pid helps to determine whether someone went behind such daemon's back, or maybe it's the daemon which "forgot"= to unfreeze after all. --=20 Mateusz Guzik -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html