* [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE
@ 2020-07-16 8:47 Qinglang Miao
2020-07-16 9:25 ` John Garry
2020-07-16 12:39 ` luojiaxing
0 siblings, 2 replies; 6+ messages in thread
From: Qinglang Miao @ 2020-07-16 8:47 UTC (permalink / raw)
To: Greg Kroah-Hartman, John Garry; +Cc: linux-scsi, linux-kernel
From: Yongqiang Liu <liuyongqiang13@huawei.com>
Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 137 ++------------------------
1 file changed, 10 insertions(+), 127 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 852d2620e..f50b0c78f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2870,19 +2870,7 @@ static int hisi_sas_debugfs_global_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_global_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_global_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_global_fops = {
- .open = hisi_sas_debugfs_global_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_global);
static int hisi_sas_debugfs_axi_show(struct seq_file *s, void *p)
{
@@ -2897,19 +2885,7 @@ static int hisi_sas_debugfs_axi_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_axi_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_axi_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_axi_fops = {
- .open = hisi_sas_debugfs_axi_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_axi);
static int hisi_sas_debugfs_ras_show(struct seq_file *s, void *p)
{
@@ -2924,19 +2900,7 @@ static int hisi_sas_debugfs_ras_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_ras_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_ras_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_ras_fops = {
- .open = hisi_sas_debugfs_ras_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_ras);
static int hisi_sas_debugfs_port_show(struct seq_file *s, void *p)
{
@@ -2951,18 +2915,7 @@ static int hisi_sas_debugfs_port_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_port_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_port_show, inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_port_fops = {
- .open = hisi_sas_debugfs_port_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_port);
static void hisi_sas_show_row_64(struct seq_file *s, int index,
int sz, __le64 *ptr)
@@ -3019,18 +2972,7 @@ static int hisi_sas_debugfs_cq_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_cq_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_cq_show, inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_cq_fops = {
- .open = hisi_sas_debugfs_cq_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_cq);
static void hisi_sas_dq_show_slot(struct seq_file *s, int slot, void *dq_ptr)
{
@@ -3052,18 +2994,7 @@ static int hisi_sas_debugfs_dq_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_dq_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_dq_show, inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_dq_fops = {
- .open = hisi_sas_debugfs_dq_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_dq);
static int hisi_sas_debugfs_iost_show(struct seq_file *s, void *p)
{
@@ -3080,18 +3011,7 @@ static int hisi_sas_debugfs_iost_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_iost_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_iost_show, inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_iost_fops = {
- .open = hisi_sas_debugfs_iost_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost);
static int hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void *p)
{
@@ -3117,20 +3037,7 @@ static int hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_iost_cache_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_iost_cache_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_iost_cache_fops = {
- .open = hisi_sas_debugfs_iost_cache_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost_cache);
static int hisi_sas_debugfs_itct_show(struct seq_file *s, void *p)
{
@@ -3147,18 +3054,7 @@ static int hisi_sas_debugfs_itct_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_itct_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_itct_show, inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_itct_fops = {
- .open = hisi_sas_debugfs_itct_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct);
static int hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void *p)
{
@@ -3184,20 +3080,7 @@ static int hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void *p)
return 0;
}
-static int hisi_sas_debugfs_itct_cache_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_itct_cache_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_itct_cache_fops = {
- .open = hisi_sas_debugfs_itct_cache_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct_cache);
static void hisi_sas_debugfs_create_files(struct hisi_hba *hisi_hba)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE
2020-07-16 8:47 [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE Qinglang Miao
@ 2020-07-16 9:25 ` John Garry
2020-07-16 13:04 ` miaoqinglang
2020-07-16 12:39 ` luojiaxing
1 sibling, 1 reply; 6+ messages in thread
From: John Garry @ 2020-07-16 9:25 UTC (permalink / raw)
To: Qinglang Miao
Cc: Greg Kroah-Hartman, linux-scsi, linux-kernel,
James E . J . Bottomley, Martin K . Petersen
On 16/07/2020 09:47, Qinglang Miao wrote:
Not sure why you cc Greg, but SCSI maintainers should have been - James
and Martin
> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>
> Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
>
> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
The sender is not the author, so your signed-off-by should also be here
(after sign off from Yongqiang Liu)
> ---
> drivers/scsi/hisi_sas/hisi_sas_main.c | 137 ++------------------------
> 1 file changed, 10 insertions(+), 127 deletions(-)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 852d2620e..f50b0c78f 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -2870,19 +2870,7 @@ static int hisi_sas_debugfs_global_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_global_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_global_show,
> - inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_global_fops = {
> - .open = hisi_sas_debugfs_global_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_global);
I couldn't see an equivalent for file_operations which have a read and
write method, and the driver has a few of those.
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE
2020-07-16 8:47 [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE Qinglang Miao
2020-07-16 9:25 ` John Garry
@ 2020-07-16 12:39 ` luojiaxing
2020-07-17 2:17 ` miaoqinglang
2020-09-19 2:32 ` miaoqinglang
1 sibling, 2 replies; 6+ messages in thread
From: luojiaxing @ 2020-07-16 12:39 UTC (permalink / raw)
To: Qinglang Miao, Greg Kroah-Hartman, John Garry; +Cc: linux-scsi, linux-kernel
Hi, Qinglang
On 2020/7/16 16:47, Qinglang Miao wrote:
> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>
> Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
>
> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
> ---
> drivers/scsi/hisi_sas/hisi_sas_main.c | 137 ++------------------------
> 1 file changed, 10 insertions(+), 127 deletions(-)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 852d2620e..f50b0c78f 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -2870,19 +2870,7 @@ static int hisi_sas_debugfs_global_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_global_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_global_show,
> - inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_global_fops = {
> - .open = hisi_sas_debugfs_global_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
I saw that your code is different from code in kernel 5.8-rc4, and it
should be as follow:
static const struct file_operations hisi_sas_debugfs_global_fops = {
.open = hisi_sas_debugfs_global_open,
.read = seq_read,
.llseek = seq_lseek,
.release = single_release,
.owner = THIS_MODULE,
};
Plus, if we use this macro directly when we write this code, it really
makes the code simpler. But if we accept the cleanup now,
we might need to consider evading compilation failures when we merge
these code back to some older kernel (e.g kernel 4.14 for centOS 7.6).
I think this marco is introduced into kernel 4.16-rc2.
So I don't see much additional benefit to us from this simplification.
But this marco is quite helpful and I think I will use it somewhere else.
Thanks
Jiaxing
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_global);
>
> static int hisi_sas_debugfs_axi_show(struct seq_file *s, void *p)
> {
> @@ -2897,19 +2885,7 @@ static int hisi_sas_debugfs_axi_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_axi_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_axi_show,
> - inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_axi_fops = {
> - .open = hisi_sas_debugfs_axi_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_axi);
>
> static int hisi_sas_debugfs_ras_show(struct seq_file *s, void *p)
> {
> @@ -2924,19 +2900,7 @@ static int hisi_sas_debugfs_ras_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_ras_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_ras_show,
> - inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_ras_fops = {
> - .open = hisi_sas_debugfs_ras_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_ras);
>
> static int hisi_sas_debugfs_port_show(struct seq_file *s, void *p)
> {
> @@ -2951,18 +2915,7 @@ static int hisi_sas_debugfs_port_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_port_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_port_show, inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_port_fops = {
> - .open = hisi_sas_debugfs_port_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_port);
>
> static void hisi_sas_show_row_64(struct seq_file *s, int index,
> int sz, __le64 *ptr)
> @@ -3019,18 +2972,7 @@ static int hisi_sas_debugfs_cq_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_cq_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_cq_show, inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_cq_fops = {
> - .open = hisi_sas_debugfs_cq_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_cq);
>
> static void hisi_sas_dq_show_slot(struct seq_file *s, int slot, void *dq_ptr)
> {
> @@ -3052,18 +2994,7 @@ static int hisi_sas_debugfs_dq_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_dq_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_dq_show, inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_dq_fops = {
> - .open = hisi_sas_debugfs_dq_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_dq);
>
> static int hisi_sas_debugfs_iost_show(struct seq_file *s, void *p)
> {
> @@ -3080,18 +3011,7 @@ static int hisi_sas_debugfs_iost_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_iost_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_iost_show, inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_iost_fops = {
> - .open = hisi_sas_debugfs_iost_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost);
>
> static int hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void *p)
> {
> @@ -3117,20 +3037,7 @@ static int hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_iost_cache_open(struct inode *inode,
> - struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_iost_cache_show,
> - inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_iost_cache_fops = {
> - .open = hisi_sas_debugfs_iost_cache_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost_cache);
>
> static int hisi_sas_debugfs_itct_show(struct seq_file *s, void *p)
> {
> @@ -3147,18 +3054,7 @@ static int hisi_sas_debugfs_itct_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_itct_open(struct inode *inode, struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_itct_show, inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_itct_fops = {
> - .open = hisi_sas_debugfs_itct_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct);
>
> static int hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void *p)
> {
> @@ -3184,20 +3080,7 @@ static int hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void *p)
> return 0;
> }
>
> -static int hisi_sas_debugfs_itct_cache_open(struct inode *inode,
> - struct file *filp)
> -{
> - return single_open(filp, hisi_sas_debugfs_itct_cache_show,
> - inode->i_private);
> -}
> -
> -static const struct file_operations hisi_sas_debugfs_itct_cache_fops = {
> - .open = hisi_sas_debugfs_itct_cache_open,
> - .read_iter = seq_read_iter,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct_cache);
>
> static void hisi_sas_debugfs_create_files(struct hisi_hba *hisi_hba)
> {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE
2020-07-16 9:25 ` John Garry
@ 2020-07-16 13:04 ` miaoqinglang
0 siblings, 0 replies; 6+ messages in thread
From: miaoqinglang @ 2020-07-16 13:04 UTC (permalink / raw)
To: John Garry
Cc: Greg Kroah-Hartman, linux-scsi, linux-kernel,
James E . J . Bottomley, Martin K . Petersen
在 2020/7/16 17:25, John Garry 写道:
> On 16/07/2020 09:47, Qinglang Miao wrote:
>
> Not sure why you cc Greg, but SCSI maintainers should have been -
> James and Martin
>
>> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>>
>> Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
>>
>> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
>
> The sender is not the author, so your signed-off-by should also be
> here (after sign off from Yongqiang Liu)
>
Sorry for not knowing this rule, I will send a new patch later on if you
don't mind.
>> ---
>> drivers/scsi/hisi_sas/hisi_sas_main.c | 137 ++------------------------
>> 1 file changed, 10 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> index 852d2620e..f50b0c78f 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> @@ -2870,19 +2870,7 @@ static int hisi_sas_debugfs_global_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_global_open(struct inode *inode,
>> struct file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_global_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_global_fops = {
>> - .open = hisi_sas_debugfs_global_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_global);
>
> I couldn't see an equivalent for file_operations which have a read and
> write method, and the driver has a few of those.
Yes, you're right. There's no equivalence when drivers have read and
write method. So the file_operations we cleaned up are those without
write method, like hisi_sas_debugfs_global_fops.
>
> Thanks
> .
Thanks
.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE
2020-07-16 12:39 ` luojiaxing
@ 2020-07-17 2:17 ` miaoqinglang
2020-09-19 2:32 ` miaoqinglang
1 sibling, 0 replies; 6+ messages in thread
From: miaoqinglang @ 2020-07-17 2:17 UTC (permalink / raw)
To: luojiaxing, Greg Kroah-Hartman, John Garry; +Cc: linux-scsi, linux-kernel
在 2020/7/16 20:39, luojiaxing 写道:
> Hi, Qinglang
>
> On 2020/7/16 16:47, Qinglang Miao wrote:
>> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>>
>> Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
>>
>> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
>> ---
>> drivers/scsi/hisi_sas/hisi_sas_main.c | 137 ++------------------------
>> 1 file changed, 10 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> index 852d2620e..f50b0c78f 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> @@ -2870,19 +2870,7 @@ static int hisi_sas_debugfs_global_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_global_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_global_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_global_fops = {
>> - .open = hisi_sas_debugfs_global_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>
>
> I saw that your code is different from code in kernel 5.8-rc4, and it
> should be as follow:
>
> static const struct file_operations hisi_sas_debugfs_global_fops = {
> .open = hisi_sas_debugfs_global_open,
> .read = seq_read,
> .llseek = seq_lseek,
> .release = single_release,
> .owner = THIS_MODULE,
> };
>
Sorry I didn't mention it in commit log, but this patch is based on
linux-next where commit <4d4901c6d7> has switched over direct seq_read
method calls to seq_read_iter. I can send a new patch based on v5.8-rc
if you don't mind.
>
> Plus, if we use this macro directly when we write this code, it really
> makes the code simpler. But if we accept the cleanup now,
>
> we might need to consider evading compilation failures when we merge
> these code back to some older kernel (e.g kernel 4.14 for centOS 7.6).
>
> I think this marco is introduced into kernel 4.16-rc2.
>
Yes, you're right, the macro and commit <4d4901c6d7> need to be applied
before this clean up patch. But I don't think this patch as well as
commit<4d4901c6d7> need to be merged back to older kernel.
>
> So I don't see much additional benefit to us from this simplification.
> But this marco is quite helpful and I think I will use it somewhere else.
>
> Thanks
>
> Jiaxing
>
Glad to know your opnions.
Thanks.
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_global);
>> static int hisi_sas_debugfs_axi_show(struct seq_file *s, void *p)
>> {
>> @@ -2897,19 +2885,7 @@ static int hisi_sas_debugfs_axi_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_axi_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_axi_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_axi_fops = {
>> - .open = hisi_sas_debugfs_axi_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_axi);
>> static int hisi_sas_debugfs_ras_show(struct seq_file *s, void *p)
>> {
>> @@ -2924,19 +2900,7 @@ static int hisi_sas_debugfs_ras_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_ras_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_ras_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_ras_fops = {
>> - .open = hisi_sas_debugfs_ras_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_ras);
>> static int hisi_sas_debugfs_port_show(struct seq_file *s, void *p)
>> {
>> @@ -2951,18 +2915,7 @@ static int hisi_sas_debugfs_port_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_port_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_port_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_port_fops = {
>> - .open = hisi_sas_debugfs_port_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_port);
>> static void hisi_sas_show_row_64(struct seq_file *s, int index,
>> int sz, __le64 *ptr)
>> @@ -3019,18 +2972,7 @@ static int hisi_sas_debugfs_cq_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_cq_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_cq_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_cq_fops = {
>> - .open = hisi_sas_debugfs_cq_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_cq);
>> static void hisi_sas_dq_show_slot(struct seq_file *s, int slot, void
>> *dq_ptr)
>> {
>> @@ -3052,18 +2994,7 @@ static int hisi_sas_debugfs_dq_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_dq_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_dq_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_dq_fops = {
>> - .open = hisi_sas_debugfs_dq_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_dq);
>> static int hisi_sas_debugfs_iost_show(struct seq_file *s, void *p)
>> {
>> @@ -3080,18 +3011,7 @@ static int hisi_sas_debugfs_iost_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_iost_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_iost_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_iost_fops = {
>> - .open = hisi_sas_debugfs_iost_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost);
>> static int hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void
>> *p)
>> {
>> @@ -3117,20 +3037,7 @@ static int
>> hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_iost_cache_open(struct inode *inode,
>> - struct file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_iost_cache_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_iost_cache_fops = {
>> - .open = hisi_sas_debugfs_iost_cache_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost_cache);
>> static int hisi_sas_debugfs_itct_show(struct seq_file *s, void *p)
>> {
>> @@ -3147,18 +3054,7 @@ static int hisi_sas_debugfs_itct_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_itct_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_itct_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_itct_fops = {
>> - .open = hisi_sas_debugfs_itct_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct);
>> static int hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void
>> *p)
>> {
>> @@ -3184,20 +3080,7 @@ static int
>> hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_itct_cache_open(struct inode *inode,
>> - struct file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_itct_cache_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_itct_cache_fops = {
>> - .open = hisi_sas_debugfs_itct_cache_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct_cache);
>> static void hisi_sas_debugfs_create_files(struct hisi_hba *hisi_hba)
>> {
>
> .
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE
2020-07-16 12:39 ` luojiaxing
2020-07-17 2:17 ` miaoqinglang
@ 2020-09-19 2:32 ` miaoqinglang
1 sibling, 0 replies; 6+ messages in thread
From: miaoqinglang @ 2020-09-19 2:32 UTC (permalink / raw)
To: luojiaxing, Greg Kroah-Hartman, John Garry; +Cc: linux-scsi, linux-kernel
在 2020/7/16 20:39, luojiaxing 写道:
> Hi, Qinglang
>
> On 2020/7/16 16:47, Qinglang Miao wrote:
>> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>>
>> Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
>>
>> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
>> ---
>> drivers/scsi/hisi_sas/hisi_sas_main.c | 137 ++------------------------
>> 1 file changed, 10 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> index 852d2620e..f50b0c78f 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> @@ -2870,19 +2870,7 @@ static int hisi_sas_debugfs_global_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_global_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_global_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_global_fops = {
>> - .open = hisi_sas_debugfs_global_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>
>
> I saw that your code is different from code in kernel 5.8-rc4, and it
> should be as follow:
>
> static const struct file_operations hisi_sas_debugfs_global_fops = {
> .open = hisi_sas_debugfs_global_open,
> .read = seq_read,
> .llseek = seq_lseek,
> .release = single_release,
> .owner = THIS_MODULE,
> };
>
>
> Plus, if we use this macro directly when we write this code, it really
> makes the code simpler. But if we accept the cleanup now,
>
> we might need to consider evading compilation failures when we merge
> these code back to some older kernel (e.g kernel 4.14 for centOS 7.6).
>
> I think this marco is introduced into kernel 4.16-rc2.
>
>
> So I don't see much additional benefit to us from this simplification.
> But this marco is quite helpful and I think I will use it somewhere else.
>
> Thanks
>
> Jiaxing
Hi Jiaxing,
I think the 'clean-up' patch means to standardize codes, which can also
make readers understand codes better.
As for 'merge back', if this situation really exists, evading
compilation failures should be necessary. But all you need to do is to
introduce the macro.
I respect your opnion anyhow. But I still resent a new patch based
against linux-next(20200917) which can be applied to mainline cleanly
now just in case you need it.
Thanks.
>
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_global);
>> static int hisi_sas_debugfs_axi_show(struct seq_file *s, void *p)
>> {
>> @@ -2897,19 +2885,7 @@ static int hisi_sas_debugfs_axi_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_axi_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_axi_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_axi_fops = {
>> - .open = hisi_sas_debugfs_axi_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_axi);
>> static int hisi_sas_debugfs_ras_show(struct seq_file *s, void *p)
>> {
>> @@ -2924,19 +2900,7 @@ static int hisi_sas_debugfs_ras_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_ras_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_ras_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_ras_fops = {
>> - .open = hisi_sas_debugfs_ras_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_ras);
>> static int hisi_sas_debugfs_port_show(struct seq_file *s, void *p)
>> {
>> @@ -2951,18 +2915,7 @@ static int hisi_sas_debugfs_port_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_port_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_port_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_port_fops = {
>> - .open = hisi_sas_debugfs_port_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_port);
>> static void hisi_sas_show_row_64(struct seq_file *s, int index,
>> int sz, __le64 *ptr)
>> @@ -3019,18 +2972,7 @@ static int hisi_sas_debugfs_cq_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_cq_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_cq_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_cq_fops = {
>> - .open = hisi_sas_debugfs_cq_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_cq);
>> static void hisi_sas_dq_show_slot(struct seq_file *s, int slot, void
>> *dq_ptr)
>> {
>> @@ -3052,18 +2994,7 @@ static int hisi_sas_debugfs_dq_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_dq_open(struct inode *inode, struct file
>> *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_dq_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_dq_fops = {
>> - .open = hisi_sas_debugfs_dq_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_dq);
>> static int hisi_sas_debugfs_iost_show(struct seq_file *s, void *p)
>> {
>> @@ -3080,18 +3011,7 @@ static int hisi_sas_debugfs_iost_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_iost_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_iost_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_iost_fops = {
>> - .open = hisi_sas_debugfs_iost_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost);
>> static int hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void
>> *p)
>> {
>> @@ -3117,20 +3037,7 @@ static int
>> hisi_sas_debugfs_iost_cache_show(struct seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_iost_cache_open(struct inode *inode,
>> - struct file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_iost_cache_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_iost_cache_fops = {
>> - .open = hisi_sas_debugfs_iost_cache_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_iost_cache);
>> static int hisi_sas_debugfs_itct_show(struct seq_file *s, void *p)
>> {
>> @@ -3147,18 +3054,7 @@ static int hisi_sas_debugfs_itct_show(struct
>> seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_itct_open(struct inode *inode, struct
>> file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_itct_show,
>> inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_itct_fops = {
>> - .open = hisi_sas_debugfs_itct_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct);
>> static int hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void
>> *p)
>> {
>> @@ -3184,20 +3080,7 @@ static int
>> hisi_sas_debugfs_itct_cache_show(struct seq_file *s, void *p)
>> return 0;
>> }
>> -static int hisi_sas_debugfs_itct_cache_open(struct inode *inode,
>> - struct file *filp)
>> -{
>> - return single_open(filp, hisi_sas_debugfs_itct_cache_show,
>> - inode->i_private);
>> -}
>> -
>> -static const struct file_operations hisi_sas_debugfs_itct_cache_fops = {
>> - .open = hisi_sas_debugfs_itct_cache_open,
>> - .read_iter = seq_read_iter,
>> - .llseek = seq_lseek,
>> - .release = single_release,
>> - .owner = THIS_MODULE,
>> -};
>> +DEFINE_SHOW_ATTRIBUTE(hisi_sas_debugfs_itct_cache);
>> static void hisi_sas_debugfs_create_files(struct hisi_hba *hisi_hba)
>> {
>
> .
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-19 2:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 8:47 [PATCH -next] scsi: hisi_sas: Convert to DEFINE_SHOW_ATTRIBUTE Qinglang Miao
2020-07-16 9:25 ` John Garry
2020-07-16 13:04 ` miaoqinglang
2020-07-16 12:39 ` luojiaxing
2020-07-17 2:17 ` miaoqinglang
2020-09-19 2:32 ` miaoqinglang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).