Re: [PATCH 2/5] xfstest: generic/219 add _require_odirect

[Dave Chinner <david@fromorbit.com>]

On Mon, Aug 03, 2015 at 01:01:22PM +0800, Dongsheng Yang wrote:
> On 08/03/2015 11:59 AM, Dave Chinner wrote:
> >On Thu, Jul 30, 2015 at 02:06:54PM +0800, Dongsheng Yang wrote:
> >>generic/219 is testing quota in three use cases including
> >>Direct I/O. So we have to add a dependence to odirect in
> >>this test.
> >
> >Hmmm - you've modified on only 2 tests here in this patch series
> >that use direct IO. There are many more - just looking for direct IO
> >comments, these generic tests use direct IO in some way, and there
> >are others that such usage is not obvious (e.g. this patch to
> >generic/219):
> >
> >generic/091
> >generic/113
> >generic/130
> >generic/214
> >generic/224
> >generic/263
> >shared/272
> >
> >And all the aio/dio tests:
> >
> >generic/036
> >generic/198
> >generic/207
> >generic/208
> >generic/209
> >generic/210
> >generic/211
> >generic/212
> >generic/239
> >generic/240
> >generic/323
> >
> >And there are others that run fio that uses DIO, too, like
> >
> >generic/299
> >generic/300
> >
> >And so on. I haven't even looked at all the fsx tests that might
> >use DIO, and I know that some of them are not listed above...
> >
> >On the whole, I think ubifs would be better to implement DIO via
> >buffered fallback that to make us have to annotate every test that
> >uses DIO in some way...
> 
> Fair point. Yes, It's difficult to find out all the tests which
> would use DIO.
> 
> And we are discussing about dio in ubifs.
> 
> So if you are strongly rejecting this kind of change, I will drop
> this patch in my next version.

I would say "strongy rejecting", more advising a different approach
to solve the problem.  i.e. with a "return 0" ->direct_IO method,
applications that use direct IO won't randomly fail or refuse to
run.  That seems like a much better (and simpler!) solution compared
to chasing down applications that use direct IO and changing
them....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com