* Re: [PATCH V2 1/1] scsi/ses: Saw "Failed to get diagnostic page 0x1"
2021-09-10 19:04 [PATCH V2 1/1] scsi/ses: Saw "Failed to get diagnostic page 0x1" wenxiong
@ 2021-09-11 0:23 ` kernel test robot
2021-09-11 0:44 ` James Bottomley
2021-09-14 15:52 ` Martin K. Petersen
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-09-11 0:23 UTC (permalink / raw)
To: wenxiong, jejb
Cc: kbuild-all, linux-scsi, brking1, martin.petersen, wenxiong,
Wen Xiong
[-- Attachment #1: Type: text/plain, Size: 6523 bytes --]
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next v5.14 next-20210910]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/wenxiong-linux-vnet-ibm-com/scsi-ses-Saw-Failed-to-get-diagnostic-page-0x1/20210911-043434
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: microblaze-buildonly-randconfig-r001-20210910 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a117eeeef2a13989a97ac0e10d86ffa6314f481e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review wenxiong-linux-vnet-ibm-com/scsi-ses-Saw-Failed-to-get-diagnostic-page-0x1/20210911-043434
git checkout a117eeeef2a13989a97ac0e10d86ffa6314f481e
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=microblaze SHELL=/bin/bash drivers/scsi/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/microblaze/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:264,
from include/asm-generic/bug.h:5,
from ./arch/microblaze/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from drivers/scsi/ses.c:8:
drivers/scsi/ses.c: In function 'ses_recv_diag':
>> include/linux/stddef.h:8:14: warning: passing argument 7 of 'scsi_execute_req' makes integer from pointer without a cast [-Wint-conversion]
8 | #define NULL ((void *)0)
| ^~~~~~~~~~~
| |
| void *
drivers/scsi/ses.c:95:42: note: in expansion of macro 'NULL'
95 | bufflen, &sshdr, NULL, SES_TIMEOUT, SES_RETRIES, NULL);
| ^~~~
In file included from include/scsi/scsi_cmnd.h:12,
from drivers/scsi/ses.c:15:
include/scsi/scsi_device.h:467:61: note: expected 'int' but argument is of type 'void *'
467 | unsigned bufflen, struct scsi_sense_hdr *sshdr, int timeout,
| ~~~~^~~~~~~
>> drivers/scsi/ses.c:61:21: warning: passing argument 9 of 'scsi_execute_req' makes pointer from integer without a cast [-Wint-conversion]
61 | #define SES_RETRIES 3
| ^
| |
| int
drivers/scsi/ses.c:95:61: note: in expansion of macro 'SES_RETRIES'
95 | bufflen, &sshdr, NULL, SES_TIMEOUT, SES_RETRIES, NULL);
| ^~~~~~~~~~~
In file included from include/scsi/scsi_cmnd.h:12,
from drivers/scsi/ses.c:15:
include/scsi/scsi_device.h:468:27: note: expected 'int *' but argument is of type 'int'
468 | int retries, int *resid)
| ~~~~~^~~~~
>> drivers/scsi/ses.c:94:24: error: too many arguments to function 'scsi_execute_req'
94 | ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf,
| ^~~~~~~~~~~~~~~~
In file included from include/scsi/scsi_cmnd.h:12,
from drivers/scsi/ses.c:15:
include/scsi/scsi_device.h:465:19: note: declared here
465 | static inline int scsi_execute_req(struct scsi_device *sdev,
| ^~~~~~~~~~~~~~~~
vim +/scsi_execute_req +94 drivers/scsi/ses.c
59
60 #define SES_TIMEOUT (30 * HZ)
> 61 #define SES_RETRIES 3
62
63 static void init_device_slot_control(unsigned char *dest_desc,
64 struct enclosure_component *ecomp,
65 unsigned char *status)
66 {
67 memcpy(dest_desc, status, 4);
68 dest_desc[0] = 0;
69 /* only clear byte 1 for ENCLOSURE_COMPONENT_DEVICE */
70 if (ecomp->type == ENCLOSURE_COMPONENT_DEVICE)
71 dest_desc[1] = 0;
72 dest_desc[2] &= 0xde;
73 dest_desc[3] &= 0x3c;
74 }
75
76
77 static int ses_recv_diag(struct scsi_device *sdev, int page_code,
78 void *buf, int bufflen)
79 {
80 int ret;
81 unsigned char cmd[] = {
82 RECEIVE_DIAGNOSTIC,
83 1, /* Set PCV bit */
84 page_code,
85 bufflen >> 8,
86 bufflen & 0xff,
87 0
88 };
89 unsigned char recv_page_code;
90 struct scsi_sense_hdr sshdr;
91 int retries = SES_RETRIES;
92
93 do {
> 94 ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf,
95 bufflen, &sshdr, NULL, SES_TIMEOUT, SES_RETRIES, NULL);
96
97 } while (scsi_sense_valid(&sshdr) &&
98 sshdr.sense_key == UNIT_ATTENTION && --retries);
99
100 if (unlikely(ret))
101 return ret;
102
103 recv_page_code = ((unsigned char *)buf)[0];
104
105 if (likely(recv_page_code == page_code))
106 return ret;
107
108 /* successful diagnostic but wrong page code. This happens to some
109 * USB devices, just print a message and pretend there was an error */
110
111 sdev_printk(KERN_ERR, sdev,
112 "Wrong diagnostic page; asked for %d got %u\n",
113 page_code, recv_page_code);
114
115 return -EINVAL;
116 }
117
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37895 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2 1/1] scsi/ses: Saw "Failed to get diagnostic page 0x1"
2021-09-10 19:04 [PATCH V2 1/1] scsi/ses: Saw "Failed to get diagnostic page 0x1" wenxiong
2021-09-11 0:23 ` kernel test robot
@ 2021-09-11 0:44 ` James Bottomley
[not found] ` <f754c31d348465f96ad4cd7541a86168@imap.linux.ibm.com>
2021-09-14 15:52 ` Martin K. Petersen
2 siblings, 1 reply; 8+ messages in thread
From: James Bottomley @ 2021-09-11 0:44 UTC (permalink / raw)
To: wenxiong; +Cc: linux-scsi, brking1, martin.petersen, wenxiong, Wen Xiong
On Fri, 2021-09-10 at 14:04 -0500, wenxiong@linux.vnet.ibm.com wrote:
> From: Wen Xiong <root@ltczz405-lp2.aus.stglabs.ibm.com>
>
> We saw two errors with Slider drawer:
> - Failed to get diagnostic page 0x1 during booting up
> - /sys/class/enclosure is empty with ipr adapter + Slider drawer
>
> From scsi logging level with error=3, looks ses_recv_diag not try on
> a UA. The patch addes retrying on a UA in ses_recv_diag();
Do we know why the device is returning a UA? Presumably it's a check
condition UA meaning the device is trying to tell us something and
wants us to request sense?
> Signed-Off-by: Wen Xiong<wenxiong@linux.vnet.ibm.com>
> Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
> ---
> drivers/scsi/ses.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index c2afba2a5414..93f6a8ce1bea 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -87,9 +87,16 @@ static int ses_recv_diag(struct scsi_device *sdev,
> int page_code,
> 0
> };
> unsigned char recv_page_code;
> + struct scsi_sense_hdr sshdr;
> + int retries = SES_RETRIES;
> +
> + do {
> + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE,
> buf,
> + bufflen, &sshdr, NULL, SES_TIMEOUT,
This grew an additional argument: you want to replace the NULL with the
sshdr, I think ... and compile test it next time.
> SES_RETRIES, NULL);
I think you want a 1 here instead of SES_RETRIES because you're
retrying for SES_RETRIES in an outer loop now, so if you maxed out both
sets of retries, you'd retry for SES_RETRIES^2.
If this is a CC/UA, you can simplify all this by setting
cmd->expecting_cc_ua = 1
and avoiding the loop.
James
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V2 1/1] scsi/ses: Saw "Failed to get diagnostic page 0x1"
2021-09-10 19:04 [PATCH V2 1/1] scsi/ses: Saw "Failed to get diagnostic page 0x1" wenxiong
2021-09-11 0:23 ` kernel test robot
2021-09-11 0:44 ` James Bottomley
@ 2021-09-14 15:52 ` Martin K. Petersen
2 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2021-09-14 15:52 UTC (permalink / raw)
To: wenxiong; +Cc: jejb, linux-scsi, brking, martin.petersen, wenxiong
Wendy,
A few small additional nits...
> From: Wen Xiong <root@ltczz405-lp2.aus.stglabs.ibm.com>
Please make sure your email address is correct.
> Signed-Off-by: Wen Xiong<wenxiong@linux.vnet.ibm.com>
This should be Signed-off-by: and you need a space before your email
address.
> + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf,
> + bufflen, &sshdr, NULL, SES_TIMEOUT, SES_RETRIES, NULL);
The sense header goes in the field before SES_TIMEOUT:
int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
int data_direction, void *buffer, unsigned bufflen,
unsigned char *sense, struct scsi_sense_hdr *sshdr,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
int timeout, int retries, u64 flags, req_flags_t rq_flags,
^^^^^^^^^^^
int *resid)
Thanks!
PS. Bonus points for whoever fixes up the scsi_execute calls to have
less than 10,000 arguments. One would be good. And some sensible input
validation/type checking.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 8+ messages in thread