On Wed, Aug 08, 2012 at 11:52:51PM -0700, Tejun Heo wrote:

On Wed, Aug 08, 2012 at 07:56:10PM -0700, Kent Overstreet wrote:

quoted

quoted

So, bio_clone() loses its function comment.  Also, does it even make
sense to call bio_clone() from fs_bio_set?

I'll re add the function comment if you want, just for a single line
wrapper I don't know if it's worth the cost - comments get out of date,
and they're more stuff to wade through.

People actually look at docbook generated docs.  I don't know why but
they do.  It's a utility function at block layer.  Please just add the
comment.

Will do then.

quoted

quoted

Let's say it's so, then
what's the difference from using _kmalloc variant?

bio_kmalloc() fails if nr_iovecs > 1024, bio_alloc_bioset() fails if
nr_iovecs > 256

and bio_alloc_bioset() is mempool backed, bio_kmalloc() is not.

AFAICT that's it.

So, the thing is being mempool backed doesn't mean anything if
multiple layers use the pool. 

It's worse than just using kmalloc, because then you've introduced the
possibility of deadlock.

I *suspect* fs_bio_set is supposed to
be used by fs layer - ie. where bios originate.  The reason why I
wondered about bio_clone() is that bio_clone() is almost always used
from stacking drivers and stacking driver tapping into fs reserve is
buggy.  So, I'm wondering whether cloning from fs_bio_set should be
supported at all.

That's actually a really good point.

I just grepped and there's actually only 3 callers - I thought there'd
be more.  That should be easy to fix, at least.