>On 7/27/07, Joe Jin <[EMAIL PROTECTED]> wrote:
> > I think we still want to run dio_cleanup() if do_direct_IO() failed?
> > Otherwise we can leak pages.
> >
> > And there's nothing special about EFAULT or ENOMEM here: if do_direct_IO()
> > returns any error then that's it: we bale out, yes?
> >
>
> Yes, I think we'll out from here if get EFAULT/ENOMEM error, also maybe -EIO
> return, return diretly should ok.
>
> > In fact I'm suspecting that this is what the code in there used to do.
> > Something like:
> >
> > for (...) {
> > ...
> > ret = do_direct_IO(...);
> > ...
> > if (ret) {
> > dio_dleanup(dio);
> > break
> > }
> > }
> > return ret;
> >
>
> Yes, we need call dio_cleanup() to release page cache, I lost it.
>
> However, we need do more while return -ENOTBLK, right?
> so I think the patch maybe like following:
>
>
> --- linux-2.6.22/fs/direct-io.c.orig 2007-07-27 14:39:15.000000000 +0800
> +++ linux-2.6.22/fs/direct-io.c 2007-07-27 15:08:58.000000000 +0800
> @@ -1032,18 +1032,19 @@ direct_io_worker(int rw, struct kiocb *i
> blkbits);
>
> if (ret) {
> + if (ret == -ENOTBLK && (rw & WRITE)) {
> + /*
> + * The remaining part of the request will be
> + * be handled by buffered I/O when we return
> + */
> + ret = 0;
> + break;
> + }
> dio_cleanup(dio);
> - break;
> + goto out;
> }
> } /* end iovec loop */
>
> - if (ret == -ENOTBLK && (rw & WRITE)) {
> - /*
> - * The remaining part of the request will be
> - * be handled by buffered I/O when we return
> - */
> - ret = 0;
> - }
> /*
> * There may be some unwritten disk at the end of a part-written
> * fs-block-sized block. Go zero that now.
>
>
>
>
> > _
> >
> > However I'd like to ask you guys to carefully review and test that please.
> >
> Gurudas, would you please give more test of this patch?
>
Hi,
If you are sure the problem is caused by EFAULT or ENOMEM , why not:
if (ret) {
dio_cleanup(dio);
if (ret == -EFAULT || ret == -ENOMEM)
goto out;
break;
}
regards
dave
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
