Re: ->direct_IO API change in current 2.4 BK

[Jeff Garzik <jgarzik@pobox.com>]

Trond Myklebust wrote:
>>>>>>" " == Jeff Garzik <jgarzik@pobox.com> writes:
> 
> 
>      > Having the stable API change, conditional on a define, is
>      > really nasty and IMO will create maintenance and support
>      > headaches down the line.  I do not recall Linux VFS _ever_
>      > having a hook's definition conditional.  We should not start
>      > now...
> 
> direct_IO() was precisely such a conditional hook definition. It
> appeared in 2.4.15, and anybody who does not check for
> KERNEL_HAS_O_DIRECT is not backward compatible.

You misunderstand.  The 2.4.15 direct_IO hook was _not_ conditionally 
defined.  It appeared in the middle of a stable series, yes.  It has a 
feature macro, yes.  But the definition of the hook in 
include/linux/fs.h does not _change_ based on a define.  That is what I 
mean by a conditional hook definition.

It is far less trouble for everyone to add a new hook, instead of 
changing an existing hook, in the middle of a stable series.


> To comment further: There is at least one example I can think of which
> was exactly equivalent to the proposed change, namely the redefinition
> of the filldir_t type in 2.4.9. It was admittedly not documented using
> a define...

No doubt you can find more :)  That doesn't make the right thing to do, 
though :)


> Note: We could at the same time replace the name direct_IO() with
> direct_IO2() (that has several precedents).  There are currently only
> a small number of filesystems that provide O_DIRECT, and converting
> them all is (as has been pointed out before) trivial...

We cannot just-fix-up filesystems which are not in-tree, which is what 
the KERNEL_HAS_O_DIRECT2 define would be mainly used for.  In-tree 
filesystems would just unconditionally use the new, or old, interface as 
they chose.


> The problem with read_inode2() was rather that it overloaded the the
> existing iget4() interface...

The higher-level problem was that we didn't want to change the VFS 
API...  otherwise we could have simply used the new interface, and 
converted all in-tree filesystems.

	Jeff