| Message ID | 155053494545.24125.14664274760505145343.stgit@noble.brown (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | More lustre patches from obdclass | expand |
> libcfs_kkuc_msg_put() is never used outside of kernelcomm.c, > so make it static. In the OpenSFS branch libcfs_kkuc_msg_put() is used by the client and server code. While this is okay now in the future this will be reverted. So do we keep it as is so it can handle server code or do this change and then later do what I added below: > Signed-off-by: NeilBrown <neilb@suse.com> > --- > .../lustre/lustre/include/lustre_kernelcomm.h | 1 - > .../staging/lustre/lustre/obdclass/kernelcomm.c | 3 +-- > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h > index 8e3a99057a38..1ed41840ab8f 100644 > --- a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h > +++ b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h > @@ -46,7 +46,6 @@ typedef int (*libcfs_kkuc_cb_t)(void *data, void *cb_arg); > > /* Kernel methods */ > void libcfs_kkuc_init(void); #ifdef CONFIG_LUSTRE_SERVER > -int libcfs_kkuc_msg_put(struct file *fp, void *payload); #endif > int libcfs_kkuc_group_put(unsigned int group, void *payload); > int libcfs_kkuc_group_add(struct file *fp, int uid, unsigned int group, > void *data, size_t data_len); > diff --git a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c > index 09d0b1ab8d1c..27870952b1f0 100644 > --- a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c > +++ b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c > @@ -49,7 +49,7 @@ > * @param payload Payload data. First field of payload is always > * struct kuc_hdr > */ #ifndef CONFIG_LUSTRE_SERVER > -int libcfs_kkuc_msg_put(struct file *filp, void *payload) #else > +static int libcfs_kkuc_msg_put(struct file *filp, void *payload) #endif > { > struct kuc_hdr *kuch = (struct kuc_hdr *)payload; > ssize_t count = kuch->kuc_msglen; > @@ -80,7 +80,6 @@ int libcfs_kkuc_msg_put(struct file *filp, void *payload) > > return rc; > } #ifdef CONFIG_LUSTER_SERVER > -EXPORT_SYMBOL(libcfs_kkuc_msg_put); #endif > /* > * Broadcast groups are global across all mounted filesystems; I do say the ifdef do look messy.
On Sun, Feb 24 2019, James Simmons wrote: >> libcfs_kkuc_msg_put() is never used outside of kernelcomm.c, >> so make it static. > > In the OpenSFS branch libcfs_kkuc_msg_put() is used by the > client and server code. While this is okay now in the future > this will be reverted. So do we keep it as is so it can handle > server code or do this change and then later do what I added > below: Where is it used? Looking in the current whamcloud master Commit f7155420024e ("LU-11208 tests: add version check to sanity tests") $ git grep libcfs_kkuc_msg_put lustre/include/lustre_kernelcomm.h:int libcfs_kkuc_msg_put(struct file *fp, void *payload); lustre/obdclass/kernelcomm.c: * libcfs_kkuc_msg_put - send an message from kernel to userspace lustre/obdclass/kernelcomm.c:int libcfs_kkuc_msg_put(struct file *filp, void *payload) lustre/obdclass/kernelcomm.c:EXPORT_SYMBOL(libcfs_kkuc_msg_put); lustre/obdclass/kernelcomm.c: rc = libcfs_kkuc_msg_put(reg->kr_fp, payload); libcfs_kkuc_msg_put() is only called from libcfs_kkuc_group_put(). What am I missing? Thanks, NeilBrown
> >> libcfs_kkuc_msg_put() is never used outside of kernelcomm.c, > >> so make it static. > > > > In the OpenSFS branch libcfs_kkuc_msg_put() is used by the > > client and server code. While this is okay now in the future > > this will be reverted. So do we keep it as is so it can handle > > server code or do this change and then later do what I added > > below: > > Where is it used? > > Looking in the current whamcloud master > Commit f7155420024e ("LU-11208 tests: add version check to sanity tests") > > $ git grep libcfs_kkuc_msg_put > lustre/include/lustre_kernelcomm.h:int libcfs_kkuc_msg_put(struct file *fp, void *payload); > lustre/obdclass/kernelcomm.c: * libcfs_kkuc_msg_put - send an message from kernel to userspace > lustre/obdclass/kernelcomm.c:int libcfs_kkuc_msg_put(struct file *filp, void *payload) > lustre/obdclass/kernelcomm.c:EXPORT_SYMBOL(libcfs_kkuc_msg_put); > lustre/obdclass/kernelcomm.c: rc = libcfs_kkuc_msg_put(reg->kr_fp, payload); > > libcfs_kkuc_msg_put() is only called from libcfs_kkuc_group_put(). > What am I missing? I'm remembering older code. You are correct. Let me give it a proper review.
> libcfs_kkuc_msg_put() is never used outside of kernelcomm.c, > so make it static. Reviewed-by: James Simmons <jsimmons@infradead.org> > Signed-off-by: NeilBrown <neilb@suse.com> > --- > .../lustre/lustre/include/lustre_kernelcomm.h | 1 - > .../staging/lustre/lustre/obdclass/kernelcomm.c | 3 +-- > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h > index 8e3a99057a38..1ed41840ab8f 100644 > --- a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h > +++ b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h > @@ -46,7 +46,6 @@ typedef int (*libcfs_kkuc_cb_t)(void *data, void *cb_arg); > > /* Kernel methods */ > void libcfs_kkuc_init(void); > -int libcfs_kkuc_msg_put(struct file *fp, void *payload); > int libcfs_kkuc_group_put(unsigned int group, void *payload); > int libcfs_kkuc_group_add(struct file *fp, int uid, unsigned int group, > void *data, size_t data_len); > diff --git a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c > index 09d0b1ab8d1c..27870952b1f0 100644 > --- a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c > +++ b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c > @@ -49,7 +49,7 @@ > * @param payload Payload data. First field of payload is always > * struct kuc_hdr > */ > -int libcfs_kkuc_msg_put(struct file *filp, void *payload) > +static int libcfs_kkuc_msg_put(struct file *filp, void *payload) > { > struct kuc_hdr *kuch = (struct kuc_hdr *)payload; > ssize_t count = kuch->kuc_msglen; > @@ -80,7 +80,6 @@ int libcfs_kkuc_msg_put(struct file *filp, void *payload) > > return rc; > } > -EXPORT_SYMBOL(libcfs_kkuc_msg_put); > > /* > * Broadcast groups are global across all mounted filesystems; > > >
diff --git a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h index 8e3a99057a38..1ed41840ab8f 100644 --- a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h +++ b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h @@ -46,7 +46,6 @@ typedef int (*libcfs_kkuc_cb_t)(void *data, void *cb_arg); /* Kernel methods */ void libcfs_kkuc_init(void); -int libcfs_kkuc_msg_put(struct file *fp, void *payload); int libcfs_kkuc_group_put(unsigned int group, void *payload); int libcfs_kkuc_group_add(struct file *fp, int uid, unsigned int group, void *data, size_t data_len); diff --git a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c index 09d0b1ab8d1c..27870952b1f0 100644 --- a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c +++ b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c @@ -49,7 +49,7 @@ * @param payload Payload data. First field of payload is always * struct kuc_hdr */ -int libcfs_kkuc_msg_put(struct file *filp, void *payload) +static int libcfs_kkuc_msg_put(struct file *filp, void *payload) { struct kuc_hdr *kuch = (struct kuc_hdr *)payload; ssize_t count = kuch->kuc_msglen; @@ -80,7 +80,6 @@ int libcfs_kkuc_msg_put(struct file *filp, void *payload) return rc; } -EXPORT_SYMBOL(libcfs_kkuc_msg_put); /* * Broadcast groups are global across all mounted filesystems;
libcfs_kkuc_msg_put() is never used outside of kernelcomm.c, so make it static. Signed-off-by: NeilBrown <neilb@suse.com> --- .../lustre/lustre/include/lustre_kernelcomm.h | 1 - .../staging/lustre/lustre/obdclass/kernelcomm.c | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-)