diff mbox

[v3,12/23] staging/rdma/hfi1: Macro code clean up

Message ID 1445869729-7507-13-git-send-email-ira.weiny@intel.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Ira Weiny Oct. 26, 2015, 2:28 p.m. UTC
From: Mitko Haralanov <mitko.haralanov@intel.com>

Clean up the context and sdma macros and move them to a more logical place in
hfi.h

Signed-off-by: Mitko Haralanov <mitko.haralanov@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/staging/rdma/hfi1/hfi.h | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

Comments

Greg KH Oct. 27, 2015, 8:19 a.m. UTC | #1
On Mon, Oct 26, 2015 at 10:28:38AM -0400, ira.weiny@intel.com wrote:
> From: Mitko Haralanov <mitko.haralanov@intel.com>
> 
> Clean up the context and sdma macros and move them to a more logical place in
> hfi.h
> 
> Signed-off-by: Mitko Haralanov <mitko.haralanov@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  drivers/staging/rdma/hfi1/hfi.h | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
> index a35213e9b500..41ad9a30149b 100644
> --- a/drivers/staging/rdma/hfi1/hfi.h
> +++ b/drivers/staging/rdma/hfi1/hfi.h
> @@ -1104,6 +1104,16 @@ struct hfi1_filedata {
>  	int rec_cpu_num;
>  };
>  
> +/* for use in system calls, where we want to know device type, etc. */
> +#define fp_to_fd(fp) ((struct hfi1_filedata *)(fp)->private_data)
> +#define ctxt_fp(fp) (fp_to_fd((fp))->uctxt)
> +#define subctxt_fp(fp) (fp_to_fd((fp))->subctxt)
> +#define tidcursor_fp(fp) (fp_to_fd((fp))->tidcursor)
> +#define user_sdma_pkt_fp(fp) (fp_to_fd((fp))->pq)
> +#define user_sdma_comp_fp(fp) (fp_to_fd((fp))->cq)
> +#define notifier_fp(fp) (fp_to_fd((fp))->mn)
> +#define rb_fp(fp) (fp_to_fd((fp))->tid_rb_root)

Ick, no, don't do this, just spell it all out (odds are you will see tht
you can make the code simpler...)  If you don't know what "cq" or "pq"
are, then name them properly.

These need to be all removed.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ira Weiny Oct. 27, 2015, 8:51 p.m. UTC | #2
On Tue, Oct 27, 2015 at 05:19:10PM +0900, Greg KH wrote:
> On Mon, Oct 26, 2015 at 10:28:38AM -0400, ira.weiny@intel.com wrote:
> > From: Mitko Haralanov <mitko.haralanov@intel.com>
> > 
> > Clean up the context and sdma macros and move them to a more logical place in
> > hfi.h
> > 
> > Signed-off-by: Mitko Haralanov <mitko.haralanov@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> >  drivers/staging/rdma/hfi1/hfi.h | 22 ++++++++++------------
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
> > index a35213e9b500..41ad9a30149b 100644
> > --- a/drivers/staging/rdma/hfi1/hfi.h
> > +++ b/drivers/staging/rdma/hfi1/hfi.h
> > @@ -1104,6 +1104,16 @@ struct hfi1_filedata {
> >  	int rec_cpu_num;
> >  };
> >  
> > +/* for use in system calls, where we want to know device type, etc. */
> > +#define fp_to_fd(fp) ((struct hfi1_filedata *)(fp)->private_data)
> > +#define ctxt_fp(fp) (fp_to_fd((fp))->uctxt)
> > +#define subctxt_fp(fp) (fp_to_fd((fp))->subctxt)
> > +#define tidcursor_fp(fp) (fp_to_fd((fp))->tidcursor)
> > +#define user_sdma_pkt_fp(fp) (fp_to_fd((fp))->pq)
> > +#define user_sdma_comp_fp(fp) (fp_to_fd((fp))->cq)
> > +#define notifier_fp(fp) (fp_to_fd((fp))->mn)
> > +#define rb_fp(fp) (fp_to_fd((fp))->tid_rb_root)
> 
> Ick, no, don't do this, just spell it all out (odds are you will see tht
> you can make the code simpler...)  If you don't know what "cq" or "pq"
> are, then name them properly.
> 
> These need to be all removed.

Ok.

Can I add the removal of these macros to the TODO list and get this patch
accepted in the interm?

Many of the patches I am queueing up to submit as well as one in this series do
not apply cleanly without this change.  It will be much easier if I can get
everything applied and then do a global clean up of these macros after the
fact.

Thanks,
Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH Oct. 27, 2015, 9:14 p.m. UTC | #3
On Tue, Oct 27, 2015 at 04:51:15PM -0400, ira.weiny wrote:
> On Tue, Oct 27, 2015 at 05:19:10PM +0900, Greg KH wrote:
> > On Mon, Oct 26, 2015 at 10:28:38AM -0400, ira.weiny@intel.com wrote:
> > > From: Mitko Haralanov <mitko.haralanov@intel.com>
> > > 
> > > Clean up the context and sdma macros and move them to a more logical place in
> > > hfi.h
> > > 
> > > Signed-off-by: Mitko Haralanov <mitko.haralanov@intel.com>
> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > > ---
> > >  drivers/staging/rdma/hfi1/hfi.h | 22 ++++++++++------------
> > >  1 file changed, 10 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
> > > index a35213e9b500..41ad9a30149b 100644
> > > --- a/drivers/staging/rdma/hfi1/hfi.h
> > > +++ b/drivers/staging/rdma/hfi1/hfi.h
> > > @@ -1104,6 +1104,16 @@ struct hfi1_filedata {
> > >  	int rec_cpu_num;
> > >  };
> > >  
> > > +/* for use in system calls, where we want to know device type, etc. */
> > > +#define fp_to_fd(fp) ((struct hfi1_filedata *)(fp)->private_data)
> > > +#define ctxt_fp(fp) (fp_to_fd((fp))->uctxt)
> > > +#define subctxt_fp(fp) (fp_to_fd((fp))->subctxt)
> > > +#define tidcursor_fp(fp) (fp_to_fd((fp))->tidcursor)
> > > +#define user_sdma_pkt_fp(fp) (fp_to_fd((fp))->pq)
> > > +#define user_sdma_comp_fp(fp) (fp_to_fd((fp))->cq)
> > > +#define notifier_fp(fp) (fp_to_fd((fp))->mn)
> > > +#define rb_fp(fp) (fp_to_fd((fp))->tid_rb_root)
> > 
> > Ick, no, don't do this, just spell it all out (odds are you will see tht
> > you can make the code simpler...)  If you don't know what "cq" or "pq"
> > are, then name them properly.
> > 
> > These need to be all removed.
> 
> Ok.
> 
> Can I add the removal of these macros to the TODO list and get this patch
> accepted in the interm?

Nope, sorry, why would I accept a known-problem patch?  Would you do
such a thing?

> Many of the patches I am queueing up to submit as well as one in this series do
> not apply cleanly without this change.  It will be much easier if I can get
> everything applied and then do a global clean up of these macros after the
> fact.

But you would have no incentive to do that if I take this patch now :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ira Weiny Oct. 28, 2015, 3:47 p.m. UTC | #4
> > Can I add the removal of these macros to the TODO list and get this patch
> > accepted in the interm?
> 
> Nope, sorry, why would I accept a known-problem patch?  Would you do
> such a thing?
> 
> > Many of the patches I am queueing up to submit as well as one in this series do
> > not apply cleanly without this change.  It will be much easier if I can get
> > everything applied and then do a global clean up of these macros after the
> > fact.
> 
> But you would have no incentive to do that if I take this patch now :)
> 

Understood.
Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
index a35213e9b500..41ad9a30149b 100644
--- a/drivers/staging/rdma/hfi1/hfi.h
+++ b/drivers/staging/rdma/hfi1/hfi.h
@@ -1104,6 +1104,16 @@  struct hfi1_filedata {
 	int rec_cpu_num;
 };
 
+/* for use in system calls, where we want to know device type, etc. */
+#define fp_to_fd(fp) ((struct hfi1_filedata *)(fp)->private_data)
+#define ctxt_fp(fp) (fp_to_fd((fp))->uctxt)
+#define subctxt_fp(fp) (fp_to_fd((fp))->subctxt)
+#define tidcursor_fp(fp) (fp_to_fd((fp))->tidcursor)
+#define user_sdma_pkt_fp(fp) (fp_to_fd((fp))->pq)
+#define user_sdma_comp_fp(fp) (fp_to_fd((fp))->cq)
+#define notifier_fp(fp) (fp_to_fd((fp))->mn)
+#define rb_fp(fp) (fp_to_fd((fp))->tid_rb_root)
+
 extern struct list_head hfi1_dev_list;
 extern spinlock_t hfi1_devs_lock;
 struct hfi1_devdata *hfi1_lookup(int unit);
@@ -1411,18 +1421,6 @@  int snoop_send_pio_handler(struct hfi1_qp *qp, struct ahg_ib_header *ibhdr,
 void snoop_inline_pio_send(struct hfi1_devdata *dd, struct pio_buf *pbuf,
 			   u64 pbc, const void *from, size_t count);
 
-/* for use in system calls, where we want to know device type, etc. */
-#define ctxt_fp(fp) \
-	(((struct hfi1_filedata *)(fp)->private_data)->uctxt)
-#define subctxt_fp(fp) \
-	(((struct hfi1_filedata *)(fp)->private_data)->subctxt)
-#define tidcursor_fp(fp) \
-	(((struct hfi1_filedata *)(fp)->private_data)->tidcursor)
-#define user_sdma_pkt_fp(fp) \
-	(((struct hfi1_filedata *)(fp)->private_data)->pq)
-#define user_sdma_comp_fp(fp) \
-	(((struct hfi1_filedata *)(fp)->private_data)->cq)
-
 static inline struct hfi1_devdata *dd_from_ppd(struct hfi1_pportdata *ppd)
 {
 	return ppd->dd;