Hi Brian,
12/13/2012 11:51 PM, Brian Foster пишет:
[cut]
Before getting into that, I made some adjustments to the original patch
to use your new fuse_io_priv structure in place of an iocb, primarily to
decouple the request submission mechanism from the type of the user
request. The highlighted differences are:
- Updated fuse_io_priv with an async field and file pointer to preserve
the current style of interface (i.e., use this instead of iocb).
- Trigger the type of request submission based on the async field.
- Reintroduce the wait in fuse_direct_IO(), but I also pulled up the
fuse_write_update_size() call out of __fuse_direct_write() to make the
separate paths more consistent.
Sounds reasonable.
This could probably use more cleanups (i.e., it morphs your fuse_io_priv
into more of a generic I/O descriptor), but illustrates the idea.
The patch you provided looks fine. Do you have any particular (further)
cleanups in mind? I'm going to resend initial patch-set with your last
patch applied. If you provide more ideas about improving it, I'll be
glad to consider and integrate them as well.
[cut]
The problem is that FUSE tends to be the only user of this 'feature'.
I'm considering one-line change of aio_complete():
- if (is_sync_kiocb(iocb)) {
+ if (!iocb->ki_ctx) {
Though, not sure whether Miklos, Al Viro, Benjamin and others will
accept it... Let's try if you're OK about this approach. At least this
will cover all fuse dio use-cases (including libaio extending file) and
makes fuse code significantly cleaner.
Interesting idea. It seems like it could work to me but I'm not very
familiar with that code and it might not be worth cluttering that
interface for this particular case.
Yes, exactly. Moreover, after looking at aio.h closer:
> struct kioctx *ki_ctx; /* may be NULL for sync ops */
I realized that that would be rather hack: sync ops doesn't use ki_ctx
for now, but this can change in the future.
I brought this up briefly with Jeff
Moyer and he pointed me at this set (I just saw zab's mail pointing this
out as well :):
https://lkml.org/lkml/2012/10/22/321
... which looks like it could help. Perhaps we could use that new type
of kiocb with a custom callback to wake a blocking thread. That said, I
wonder if we could do that anyways; i.e., define our own
wait_on_fuse_async() type function that that waits on the number of
in-flight requests in the fuse_priv_io. I guess we'd have to complicate
the refcounting with that type of approach, though.
I've considered using that new API (kernel aio) but refrained from it
deliberately. The reasons were similar to ones you listed above: why
should we invent another synchronization mechanism if
wait_on_sync_kiocb() does exactly what we need? Now, after more
thinking, I tend to think that using that API is doable, but the way how
we could do it is too clumsy. Please let me know if you need more details.
Thanks,
Maxim
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
