2018-02-19 14:09:06

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH V3] lib/scatterlist: Add SG_CHAIN and SG_LAST macros for LSB encodings

This replaces scatterlist->page_link LSB encodings with SG_CHAIN and
SG_LAST definitions without any functional change. While here also
add macro definitions SG_PAGE_BITS and SG_PAGE_MASK.

Signed-off-by: Anshuman Khandual <[email protected]>
---
Changes in V3:
- Changed SG_END as SG_LAST due to a build failure
- Added SG_PAGE_BITS and SG_PAGE_MASK as per Tvrtko
- Used BIT() macro as per Chris

Changes in V2:
- Changed SG_EMARK as SG_END as per Johannes and Tvrtko
- Added 'UL' to the constants as per Bart

include/linux/scatterlist.h | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 22b2131bcdcd..494b84c7f3c8 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -66,15 +66,20 @@ struct sg_table {

#define SG_MAGIC 0x87654321

+#define SG_CHAIN BIT(0)
+#define SG_LAST BIT(1)
+#define SG_PAGE_BITS (SG_CHAIN | SG_LAST)
+#define SG_PAGE_MASK (~SG_PAGE_BITS)
+
/*
* We overload the LSB of the page pointer to indicate whether it's
* a valid sg entry, or whether it points to the start of a new scatterlist.
* Those low bits are there for everyone! (thanks mason :-)
*/
-#define sg_is_chain(sg) ((sg)->page_link & 0x01)
-#define sg_is_last(sg) ((sg)->page_link & 0x02)
+#define sg_is_chain(sg) ((sg)->page_link & SG_CHAIN)
+#define sg_is_last(sg) ((sg)->page_link & SG_LAST)
#define sg_chain_ptr(sg) \
- ((struct scatterlist *) ((sg)->page_link & ~0x03))
+ ((struct scatterlist *) ((sg)->page_link & SG_PAGE_MASK))

/**
* sg_assign_page - Assign a given page to an SG entry
@@ -88,13 +93,13 @@ struct sg_table {
**/
static inline void sg_assign_page(struct scatterlist *sg, struct page *page)
{
- unsigned long page_link = sg->page_link & 0x3;
+ unsigned long page_link = sg->page_link & SG_PAGE_BITS;

/*
* In order for the low bit stealing approach to work, pages
* must be aligned at a 32-bit boundary as a minimum.
*/
- BUG_ON((unsigned long) page & 0x03);
+ BUG_ON((unsigned long) page & SG_PAGE_BITS);
#ifdef CONFIG_DEBUG_SG
BUG_ON(sg->sg_magic != SG_MAGIC);
BUG_ON(sg_is_chain(sg));
@@ -130,7 +135,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
BUG_ON(sg->sg_magic != SG_MAGIC);
BUG_ON(sg_is_chain(sg));
#endif
- return (struct page *)((sg)->page_link & ~0x3);
+ return (struct page *)((sg)->page_link & SG_PAGE_MASK);
}

/**
@@ -178,7 +183,8 @@ static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
* Set lowest bit to indicate a link pointer, and make sure to clear
* the termination bit if it happens to be set.
*/
- prv[prv_nents - 1].page_link = ((unsigned long) sgl | 0x01) & ~0x02;
+ prv[prv_nents - 1].page_link = ((unsigned long) sgl | SG_CHAIN)
+ & ~SG_LAST;
}

/**
@@ -198,8 +204,8 @@ static inline void sg_mark_end(struct scatterlist *sg)
/*
* Set termination bit, clear potential chain bit
*/
- sg->page_link |= 0x02;
- sg->page_link &= ~0x01;
+ sg->page_link |= SG_LAST;
+ sg->page_link &= ~SG_CHAIN;
}

/**
@@ -215,7 +221,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
#ifdef CONFIG_DEBUG_SG
BUG_ON(sg->sg_magic != SG_MAGIC);
#endif
- sg->page_link &= ~0x02;
+ sg->page_link &= ~SG_LAST;
}

/**
--
2.11.0



2018-02-19 14:50:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V3] lib/scatterlist: Add SG_CHAIN and SG_LAST macros for LSB encodings

On Mon, Feb 19, 2018 at 4:07 PM, Anshuman Khandual
<[email protected]> wrote:
> This replaces scatterlist->page_link LSB encodings with SG_CHAIN and
> SG_LAST definitions without any functional change. While here also
> add macro definitions SG_PAGE_BITS and SG_PAGE_MASK.
>

A nit below, in any case

Reviewed-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> Changes in V3:
> - Changed SG_END as SG_LAST due to a build failure
> - Added SG_PAGE_BITS and SG_PAGE_MASK as per Tvrtko
> - Used BIT() macro as per Chris
>
> Changes in V2:
> - Changed SG_EMARK as SG_END as per Johannes and Tvrtko
> - Added 'UL' to the constants as per Bart
>
> include/linux/scatterlist.h | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 22b2131bcdcd..494b84c7f3c8 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -66,15 +66,20 @@ struct sg_table {
>
> #define SG_MAGIC 0x87654321
>
> +#define SG_CHAIN BIT(0)
> +#define SG_LAST BIT(1)
> +#define SG_PAGE_BITS (SG_CHAIN | SG_LAST)
> +#define SG_PAGE_MASK (~SG_PAGE_BITS)
> +
> /*
> * We overload the LSB of the page pointer to indicate whether it's
> * a valid sg entry, or whether it points to the start of a new scatterlist.
> * Those low bits are there for everyone! (thanks mason :-)
> */
> -#define sg_is_chain(sg) ((sg)->page_link & 0x01)
> -#define sg_is_last(sg) ((sg)->page_link & 0x02)
> +#define sg_is_chain(sg) ((sg)->page_link & SG_CHAIN)
> +#define sg_is_last(sg) ((sg)->page_link & SG_LAST)
> #define sg_chain_ptr(sg) \
> - ((struct scatterlist *) ((sg)->page_link & ~0x03))
> + ((struct scatterlist *) ((sg)->page_link & SG_PAGE_MASK))
>
> /**
> * sg_assign_page - Assign a given page to an SG entry
> @@ -88,13 +93,13 @@ struct sg_table {
> **/
> static inline void sg_assign_page(struct scatterlist *sg, struct page *page)
> {
> - unsigned long page_link = sg->page_link & 0x3;
> + unsigned long page_link = sg->page_link & SG_PAGE_BITS;
>
> /*
> * In order for the low bit stealing approach to work, pages
> * must be aligned at a 32-bit boundary as a minimum.
> */
> - BUG_ON((unsigned long) page & 0x03);
> + BUG_ON((unsigned long) page & SG_PAGE_BITS);
> #ifdef CONFIG_DEBUG_SG
> BUG_ON(sg->sg_magic != SG_MAGIC);
> BUG_ON(sg_is_chain(sg));
> @@ -130,7 +135,7 @@ static inline struct page *sg_page(struct scatterlist *sg)
> BUG_ON(sg->sg_magic != SG_MAGIC);
> BUG_ON(sg_is_chain(sg));
> #endif
> - return (struct page *)((sg)->page_link & ~0x3);
> + return (struct page *)((sg)->page_link & SG_PAGE_MASK);
> }
>
> /**
> @@ -178,7 +183,8 @@ static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
> * Set lowest bit to indicate a link pointer, and make sure to clear
> * the termination bit if it happens to be set.
> */
> - prv[prv_nents - 1].page_link = ((unsigned long) sgl | 0x01) & ~0x02;
> + prv[prv_nents - 1].page_link = ((unsigned long) sgl | SG_CHAIN)
> + & ~SG_LAST;

If it's ~2-3 characters over 80, I would rather still keep on the same line.

> }
>
> /**
> @@ -198,8 +204,8 @@ static inline void sg_mark_end(struct scatterlist *sg)
> /*
> * Set termination bit, clear potential chain bit
> */
> - sg->page_link |= 0x02;
> - sg->page_link &= ~0x01;
> + sg->page_link |= SG_LAST;
> + sg->page_link &= ~SG_CHAIN;
> }
>
> /**
> @@ -215,7 +221,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
> #ifdef CONFIG_DEBUG_SG
> BUG_ON(sg->sg_magic != SG_MAGIC);
> #endif
> - sg->page_link &= ~0x02;
> + sg->page_link &= ~SG_LAST;
> }
>
> /**
> --
> 2.11.0
>



--
With Best Regards,
Andy Shevchenko