| Message ID | 20230420164928.237235-6-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/sched: cleanup parsing prints in htb and qfq | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 8 this patch: 8 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 16 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Thu, Apr 20, 2023 at 01:49:28PM -0300, Pedro Tammela wrote: > For the sake of readability, change these two definitions to BIT() > macros. > > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
On Thu, Apr 20, 2023 at 6:50 PM Pedro Tammela <pctammela@mojatatu.com> wrote: > > For the sake of readability, change these two definitions to BIT() > macros. > > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > --- > net/sched/sch_qfq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > index dfd9a99e6257..4b9cc8a46e2a 100644 > --- a/net/sched/sch_qfq.c > +++ b/net/sched/sch_qfq.c > @@ -105,7 +105,7 @@ > #define QFQ_MAX_INDEX 24 > #define QFQ_MAX_WSHIFT 10 > > -#define QFQ_MAX_WEIGHT (1<<QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ > +#define QFQ_MAX_WEIGHT BIT(QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ I am not sure I find BIT(X) more readable in this context. Say MAX_WEIGHT was 0xF000, should we then use #define MAX_WEIGHT (BIT(15) | BIT(14) |BIT(13) | BIT(12))
On Fri, Apr 21, 2023 at 11:17:23AM +0200, Eric Dumazet wrote: > On Thu, Apr 20, 2023 at 6:50 PM Pedro Tammela <pctammela@mojatatu.com> wrote: > > > > For the sake of readability, change these two definitions to BIT() > > macros. > > > > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > > --- > > net/sched/sch_qfq.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > > index dfd9a99e6257..4b9cc8a46e2a 100644 > > --- a/net/sched/sch_qfq.c > > +++ b/net/sched/sch_qfq.c > > @@ -105,7 +105,7 @@ > > #define QFQ_MAX_INDEX 24 > > #define QFQ_MAX_WSHIFT 10 > > > > -#define QFQ_MAX_WEIGHT (1<<QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ > > +#define QFQ_MAX_WEIGHT BIT(QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ > > I am not sure I find BIT(X) more readable in this context. > > Say MAX_WEIGHT was 0xF000, should we then use > > #define MAX_WEIGHT (BIT(15) | BIT(14) |BIT(13) | BIT(12)) Thanks Eric, I think this is my mistake for suggesting this change. I agree BIT() is not so good here after all.
On 21/04/2023 06:30, Simon Horman wrote: > On Fri, Apr 21, 2023 at 11:17:23AM +0200, Eric Dumazet wrote: >> On Thu, Apr 20, 2023 at 6:50 PM Pedro Tammela <pctammela@mojatatu.com> wrote: >>> >>> For the sake of readability, change these two definitions to BIT() >>> macros. >>> >>> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> >>> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> >>> --- >>> net/sched/sch_qfq.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c >>> index dfd9a99e6257..4b9cc8a46e2a 100644 >>> --- a/net/sched/sch_qfq.c >>> +++ b/net/sched/sch_qfq.c >>> @@ -105,7 +105,7 @@ >>> #define QFQ_MAX_INDEX 24 >>> #define QFQ_MAX_WSHIFT 10 >>> >>> -#define QFQ_MAX_WEIGHT (1<<QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ >>> +#define QFQ_MAX_WEIGHT BIT(QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ >> >> I am not sure I find BIT(X) more readable in this context. >> >> Say MAX_WEIGHT was 0xF000, should we then use >> >> #define MAX_WEIGHT (BIT(15) | BIT(14) |BIT(13) | BIT(12)) > > Thanks Eric, > > I think this is my mistake for suggesting this change. > I agree BIT() is not so good here after all. Fair enough, Will remove it and repost. Thanks for the reviews.
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c index dfd9a99e6257..4b9cc8a46e2a 100644 --- a/net/sched/sch_qfq.c +++ b/net/sched/sch_qfq.c @@ -105,7 +105,7 @@ #define QFQ_MAX_INDEX 24 #define QFQ_MAX_WSHIFT 10 -#define QFQ_MAX_WEIGHT (1<<QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ +#define QFQ_MAX_WEIGHT BIT(QFQ_MAX_WSHIFT) /* see qfq_slot_insert */ #define QFQ_MAX_WSUM (64*QFQ_MAX_WEIGHT) #define FRAC_BITS 30 /* fixed point arithmetic */ @@ -113,7 +113,7 @@ #define QFQ_MTU_SHIFT 16 /* to support TSO/GSO */ #define QFQ_MIN_LMAX 512 /* see qfq_slot_insert */ -#define QFQ_MAX_LMAX (1UL << QFQ_MTU_SHIFT) +#define QFQ_MAX_LMAX BIT(QFQ_MTU_SHIFT) #define QFQ_MAX_AGG_CLASSES 8 /* max num classes per aggregate allowed */