bio: have bio_kmap_irq return the size of mapped data (fwd)

Commit Message

Mikulas Patocka Nov. 7, 2017, 9:45 p.m. UTC

Hi

I need the function bio_kmap_irq in the driver that I am developing, but 
it doesn't return the size of the mapped data. I've made this patch to fix 
it.

Mikulas


From: Mikulas Patocka <[e-mail]>

The function bio_kmap_irq is not usable because it does not return the
size of the mapped data.

Fix bio_kmap_irq and __bio_kmap_irq so that they return the size of
mapped data.

Signed-off-by: Mikulas Patocka <[e-mail]>
Signed-off-by: Mike Snitzer <[e-mail]>
---
 include/linux/bio.h |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)


--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Christoph Hellwig Nov. 8, 2017, 9:47 a.m. UTC | #1

On Tue, Nov 07, 2017 at 04:45:17PM -0500, Mikulas Patocka wrote:
> Hi
> 
> I need the function bio_kmap_irq in the driver that I am developing, but 
> it doesn't return the size of the mapped data. I've made this patch to fix 
> it.

To be honest I think we should just remove bio_kmap_irq.  It is currently
unused and assumes there is only a single bvec to map.

I think you're much better off using a proper bvec iterator in your
caller and then call bvec_kmap_irq/bvec_kunmap_irq manually.

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Mikulas Patocka Nov. 8, 2017, 12:38 p.m. UTC | #2

On Wed, 8 Nov 2017, Christoph Hellwig wrote:

> On Tue, Nov 07, 2017 at 04:45:17PM -0500, Mikulas Patocka wrote:
> > Hi
> > 
> > I need the function bio_kmap_irq in the driver that I am developing, but 
> > it doesn't return the size of the mapped data. I've made this patch to fix 
> > it.
> 
> To be honest I think we should just remove bio_kmap_irq.  It is currently
> unused and assumes there is only a single bvec to map.

It could be removed from include/linux/bio.h and moved to my driver. But 
if we leave it in bio.h, it could be used by others as well.

bio_kmap_irq can iterate over the whole bio if we use bio_advance on the 
bio.

> I think you're much better off using a proper bvec iterator in your
> caller and then call bvec_kmap_irq/bvec_kunmap_irq manually.

Mikulas

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Christoph Hellwig Nov. 8, 2017, 3:05 p.m. UTC | #3

On Wed, Nov 08, 2017 at 07:38:44AM -0500, Mikulas Patocka wrote:
> > To be honest I think we should just remove bio_kmap_irq.  It is currently
> > unused and assumes there is only a single bvec to map.
> 
> It could be removed from include/linux/bio.h and moved to my driver. But 
> if we leave it in bio.h, it could be used by others as well.
> 
> bio_kmap_irq can iterate over the whole bio if we use bio_advance on the 
> bio.

The bio_kmap_irq name implies it maps the whole bio, but it only maps
the current segment.  Now if there were plenty of users and it was
non-trivial we could say we should fix the name and documentation.
But given how trivial it is I'd rather have you open code it to clearly
document what you are doing.

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Patch

Index: linux-2.6/include/linux/bio.h
===================================================================
--- linux-2.6.orig/include/linux/bio.h
+++ linux-2.6/include/linux/bio.h
@@ -576,14 +576,16 @@  static inline void bvec_kunmap_irq(char
 #endif
 
 static inline char *__bio_kmap_irq(struct bio *bio, struct bvec_iter iter,
-				   unsigned long *flags)
+				   unsigned long *flags, unsigned *size)
 {
-	return bvec_kmap_irq(&bio_iter_iovec(bio, iter), flags);
+	struct bio_vec bv = bio_iter_iovec(bio, iter);
+	*size = bv.bv_len;
+	return bvec_kmap_irq(&bv, flags);
 }
 #define __bio_kunmap_irq(buf, flags)	bvec_kunmap_irq(buf, flags)
 
-#define bio_kmap_irq(bio, flags) \
-	__bio_kmap_irq((bio), (bio)->bi_iter, (flags))
+#define bio_kmap_irq(bio, flags, size) \
+	__bio_kmap_irq((bio), (bio)->bi_iter, (flags), (size))
 #define bio_kunmap_irq(buf,flags)	__bio_kunmap_irq(buf, flags)
 
 /*