2007-10-27 17:02:51

by Cyrill Gorcunov

Subject: [PATCH] SG: set names for numeric constants

This patch defines names for numeric constants that
used in scatter list for more convenient code reading.

Signed-off-by: Cyrill Gorcunov <[email protected]>

---

I'm not sure if it really needed but who knows ;)
Any comments are welcome.

include/linux/scatterlist.h | 21 ++++++++++++---------
1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 4571231..fcc05a5 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -25,6 +25,9 @@
*/

#define SG_MAGIC 0x87654321
+#define SG_CHAIN 0x1
+#define SG_LAST 0x2
+#define SG_MASK 0x3

/**
* sg_assign_page - Assign a given page to an SG entry
@@ -38,13 +41,13 @@
**/
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_MASK;

/*
* 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_MASK);
#ifdef CONFIG_DEBUG_SG
BUG_ON(sg->sg_magic != SG_MAGIC);
#endif
@@ -73,7 +76,7 @@ static inline void sg_set_page(struct scatterlist *sg, struct page *page,
sg->length = len;
}

-#define sg_page(sg) ((struct page *) ((sg)->page_link & ~0x3))
+#define sg_page(sg) ((struct page *) ((sg)->page_link & ~SG_MASK))

/**
* sg_set_buf - Set sg entry to point at given data
@@ -93,10 +96,10 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
* 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_MASK))

/**
* sg_next - return the next scatterlist entry in a list
@@ -179,7 +182,7 @@ static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
#ifndef ARCH_HAS_SG_CHAIN
BUG();
#endif
- prv[prv_nents - 1].page_link = (unsigned long) sgl | 0x01;
+ prv[prv_nents - 1].page_link = (unsigned long) sgl | SG_CHAIN;
}

/**
@@ -193,12 +196,12 @@ static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
**/
static inline void sg_mark_end(struct scatterlist *sgl, unsigned int nents)
{
- sgl[nents - 1].page_link = 0x02;
+ sgl[nents - 1].page_link = SG_LAST;
}

static inline void __sg_mark_end(struct scatterlist *sg)
{
- sg->page_link |= 0x02;
+ sg->page_link |= SG_LAST;
}

/**

2007-10-27 17:17:37

by Robert P. J. Day

Subject: Re: [PATCH] SG: set names for numeric constants

On Sat, 27 Oct 2007, Cyrill Gorcunov wrote:

>
> This patch defines names for numeric constants that
> used in scatter list for more convenient code reading.
>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
...
> @@ -73,7 +76,7 @@ static inline void sg_set_page(struct scatterlist *sg, struct page *page,
> sg->length = len;
> }
>
> -#define sg_page(sg) ((struct page *) ((sg)->page_link & ~0x3))
> +#define sg_page(sg) ((struct page *) ((sg)->page_link & ~SG_MASK))
>
> -#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_MASK))

while you're at it, could you move all those macros to the top of the
file, rather than leaving them scattered across the first 100 lines or
so? it would just make them easier to find when you're perusing the
code.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-10-27 17:47:16

by Cyrill Gorcunov

Subject: Re: [PATCH] SG: set names for numeric constants

[Robert P. J. Day - Sat, Oct 27, 2007 at 01:14:22PM -0400]
| On Sat, 27 Oct 2007, Cyrill Gorcunov wrote:
|
| >
| > This patch defines names for numeric constants that
| > used in scatter list for more convenient code reading.
| >
| > Signed-off-by: Cyrill Gorcunov <[email protected]>
| ...
| > @@ -73,7 +76,7 @@ static inline void sg_set_page(struct scatterlist *sg, struct page *page,
| > sg->length = len;
| > }
| >
| > -#define sg_page(sg) ((struct page *) ((sg)->page_link & ~0x3))
| > +#define sg_page(sg) ((struct page *) ((sg)->page_link & ~SG_MASK))
| >
| > -#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_MASK))
|
| while you're at it, could you move all those macros to the top of the
| file, rather than leaving them scattered across the first 100 lines or
| so? it would just make them easier to find when you're perusing the
| code.
|
| rday
| --
| ========================================================================
| Robert P. J. Day
| Linux Consulting, Training and Annoying Kernel Pedantry
| Waterloo, Ontario, CANADA
|
| http://crashcourse.ca
| ========================================================================
|

This patch defines names for numeric constants that
used in scatter list for more convenient code reading.
Also groups all macroses on the top of the file.

Signed-off-by: Cyrill Gorcunov <[email protected]>
---

Robert, what about this one?

include/linux/scatterlist.h | 47 ++++++++++++++++++++++---------------------
1 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 4571231..3edd6ca 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -25,6 +25,25 @@
*/

#define SG_MAGIC 0x87654321
+#define SG_CHAIN 0x1
+#define SG_LAST 0x2
+#define SG_MASK 0x3
+
+/*
+ * 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 & SG_CHAIN)
+#define sg_is_last(sg) ((sg)->page_link & SG_LAST)
+#define sg_chain_ptr(sg) ((struct scatterlist *)((sg)->page_link & ~SG_MASK))
+#define sg_page(sg) ((struct page *)((sg)->page_link & ~SG_MASK))
+
+/*
+ * Loop over each sg element, following the pointer to a new list if necessary
+ */
+#define for_each_sg(sglist, sg, nr, __i) \
+ for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))

/**
* sg_assign_page - Assign a given page to an SG entry
@@ -38,13 +57,13 @@
**/
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_MASK;

/*
* 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_MASK);
#ifdef CONFIG_DEBUG_SG
BUG_ON(sg->sg_magic != SG_MAGIC);
#endif
@@ -73,8 +92,6 @@ static inline void sg_set_page(struct scatterlist *sg, struct page *page,
sg->length = len;
}

-#define sg_page(sg) ((struct page *) ((sg)->page_link & ~0x3))
-
/**
* sg_set_buf - Set sg entry to point at given data
* @sg: SG entry
@@ -88,16 +105,6 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
}

-/*
- * 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_chain_ptr(sg) \
- ((struct scatterlist *) ((sg)->page_link & ~0x03))
-
/**
* sg_next - return the next scatterlist entry in a list
* @sg: The current sg entry
@@ -123,12 +130,6 @@ static inline struct scatterlist *sg_next(struct scatterlist *sg)
return sg;
}

-/*
- * Loop over each sg element, following the pointer to a new list if necessary
- */
-#define for_each_sg(sglist, sg, nr, __i) \
- for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
-
/**
* sg_last - return the last scatterlist entry in a list
* @sgl: First entry in the scatterlist
@@ -179,7 +180,7 @@ static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
#ifndef ARCH_HAS_SG_CHAIN
BUG();
#endif
- prv[prv_nents - 1].page_link = (unsigned long) sgl | 0x01;
+ prv[prv_nents - 1].page_link = (unsigned long) sgl | SG_CHAIN;
}

/**
@@ -193,12 +194,12 @@ static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
**/
static inline void sg_mark_end(struct scatterlist *sgl, unsigned int nents)
{
- sgl[nents - 1].page_link = 0x02;
+ sgl[nents - 1].page_link = SG_LAST;
}

static inline void __sg_mark_end(struct scatterlist *sg)
{
- sg->page_link |= 0x02;
+ sg->page_link |= SG_LAST;
}

/**