From mboxrd@z Thu Jan 1 00:00:00 1970 From: jorg@ml1.net Subject: Re: [PATCH take 3][RFC] fat: Save FAT root directory timestamps to volume label Date: Fri, 18 Sep 2009 15:14:10 +0300 Message-ID: <1253276050.9376.1335476075@webmail.messagingengine.com> References: <1252420131.5782.2.camel@jorg-desktop><1252420448-7282-1-git-send-email-ext-jorg.2.schummer@nokia.com> <878wgcsd1p.fsf@devron.myhome.or.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org To: "OGAWA Hirofumi" , "Jorg Schummer" Return-path: In-Reply-To: <878wgcsd1p.fsf@devron.myhome.or.jp> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Hello and thanks for your comments! On Fri, 18 Sep 2009 20:33 +0900, "OGAWA Hirofumi" wrote: > Jorg Schummer writes: >=20 > > Standard FAT implementations cannot store any of the FAT root direc= tory's > > timestamps. This commit adds the mount option 'rootts', which allow= s saving > > the FAT root directory timestamps as the timestamps of the FAT volu= me label > > directory entry. At least Mac OS X is known to support the same mec= hanism > > and interoperate with this commit. > > > > When mounting, the following values can be specified for the 'roott= s' mount > > option: > > =2E.. > > > > "rootts=3Dsave" tries to load and save the root directory's t= imestamps. > > The mtime and atime are corrected based on root > > directory entries' ctime. If the root directory= was > > accessed but no volume label entry exists, the = label > > "NO NAME" is created. >=20 > Um..., I'm still not sure though. rootts=3Dsave may not be needed for > this, because it seems a bit complex, and it can do on userland... It's true, that is quite of a hack after all. I just thought I'll try t= o add the non-plus-ultra-feature on top of what Mac OS can do already. But you're right, I'll get rid of the label creation. (And I think I should get rid of the time stamp correction as well. If there is no timestamp to be loaded, then 1970/1/1 should suffice. In case anyone thinks that the timestamp 'correction' is anyhow useful, let me know...= ) >=20 > And some quick review of implement.. >=20 > > + if (inode->i_ino =3D=3D MSDOS_ROOT_INO) { >=20 > In this path, we don't have inode->i_mutex, so to look directry up is > wrong. I think adding label may move to userland. >=20 > I guess we should do all of preparation in fill_super(). True. This code is only needed for the time stamp creation, so it'll go in any case. >=20 > > + remove_inode_hash(inode); > > + fat_attach(inode, i_pos); > > + insert_inode_hash(inode); > > + brelse(bh); >=20 > Root inode should never change the position, so this shouldn't be > needed. Like above, this will be removed. If 'preserve' (i.e. saving and loading the timestamp only IF a label is present) is the default, I'll remove the 'rootts=3Dpreserve' option. However, should there still be an option to switch this feature off completely ('rootts=3Dignore')? Thanks very much for your comments, the next version will come soon! J=F6rg