linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Christoph Hellwig <hch@lst.de>,
	chris.mason@oracle.com, linux-fsdevel@vger.kernel.org
Subject: Re: vfat vs msdos and -o flush
Date: Sun, 7 Mar 2010 14:06:42 +0100	[thread overview]
Message-ID: <20100307130642.GA32710@lst.de> (raw)
In-Reply-To: <877hpt1q96.fsf@devron.myhome.or.jp>

On Wed, Mar 03, 2010 at 10:36:21PM +0900, OGAWA Hirofumi wrote:
> >   - having the highlevel inode operations duplicated between msdos
> >     and vfat is probably a bad idea, and we should only branch out
> >     for the very low-level directory entry manipulations.
> 
> I know you are finding the reason to merge those. ;)
> 
> Well, what code are you suggesting? (BTW, this is not meaning "Please
> send a patch.").  Can we do it without the user visible change? And if
> it has the user visible change, what is benefit to users?
> 
> I know, if we kill original design decision with the user visible
> change, we can write much more faster/cleaner code. (e.g. some I/O can
> be reduced for directory. etc.) But, for fatfs, I'm thinking there is no
> benefit to take a pain by the user visible change, at least for now.

There's a couple of things I have in mind:

 - first move fat and msdos into the same module to make integration
   easier.  Keep MODULE_ALIASes for vfat and msdos to allow old module
   (auto-)load setups working.  Move the setup for the two
   file_system_type structures to super.c
 - look at the spurious differences between namei_msdos.c and
   namei_vfat.c.  Here's a few I've noticed and I'll have patches for
   soon hopefully:

     - msdos only updates c and mtime in msdos_add_entry, but vfat
       also updates atime
     - msdos only updates ctime in rmdir and unlink , vfat otoh only
       updates mtime and atime
     - vfat updates the ctime on the new inode in rename, msdos
       doesn't.

  - udate i_version consistently in msdos, too (probably not needed
    without nfs exporting, but just to make the code similar)
  - unify the vfat_add_entry/msdos_add_entry prototypes and make them
    function pointers hanging off the superblock information
  - move the hidden file handling and msdos_format_name into a helper
    and make that a function pointer hanging off the superblock,
    with a no-op implementation for vfat
  - unify the prototypes of msdos_find and vfat_find and make it
    a function pointer hanging off the superblock

At this point basically all the inode operations are the same for
both and can be merged, spreading out via the three operations
mentioned above.  If we don't want the indirection it could also
be done using if/else statements in a small helper.  The only one
I haven't analyzed in detail yet is rename, because it looks too
different and will need some refactoring first.


  parent reply	other threads:[~2010-03-07 13:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-01 17:08 vfat vs msdos and -o flush Christoph Hellwig
2010-03-03 13:36 ` OGAWA Hirofumi
2010-03-07 11:00   ` OGAWA Hirofumi
2010-03-07 13:06   ` Christoph Hellwig [this message]
2010-03-07 14:15     ` OGAWA Hirofumi
2010-03-09 12:57       ` Christoph Hellwig
2010-03-09 13:42   ` Chris Mason
2010-03-10 19:19     ` Jeff Mahoney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100307130642.GA32710@lst.de \
    --to=hch@lst.de \
    --cc=chris.mason@oracle.com \
    --cc=hirofumi@mail.parknet.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).