RE: [PATCH] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize
Yes, we normally use 128, as this is the value best suited
for our RAID systems. After upgrading the software we saw
a clear performance reduction that was caused by the bug.
With the fix everything works fine again.
Thank you,
Bodo
-Original Message-
From: James Smart [mailto:james.sm...@avagotech.com]
Sent: Friday, May 29, 2015 4:05 PM
To: Strösser, Bodo; linux-scsi@vger.kernel.org
Subject: Re: [PATCH] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize
Bodo,
This is a valid fix. Thank you.
I assume you were changing the driver module parameter to up the SG
count ? without the fix we would have been capped at 64 sges.
-- james
Reviewed-By: James Smart james.sm...@avagotech.com
On 5/29/2015 8:22 AM, bstroes...@ts.fujitsu.com wrote:
Hi James,
We had some performance problems with RAID systems connected to LPe12k.
AFAICS, the reason is a small bug in lpfc.ko, causing the IO-size to
be smaller than expected.
The patch below fixes it for us.
Please CC me, I'm not on the list.
Best regards
Bodo
--
From: Bodo Stroesser bstroes...@ts.fujitsu.com
Date: Fri, 29 May 2015 13:34:19 +0200
Subject: [PATCH] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize
Currently the module parameter lpfc_sg_seg_count does not have effect
for sli3 devices.
In lpfc_sli_driver_resource_setup(), which is used for sli3, the code
writes the configured sg_seg_cnt into lpfc_template.sg_tablesize.
But lpfc_template is the template used for sli4 only. Thus the value should
correctly be written to lpfc_template_s3-sg_tablesize.
This patch is for kernel 4.1-rc5, but is tested with lpfc 10.2.405.26 only.
Signed-off-by: Bodo Stroesser bstroes...@ts.fujitsu.com
---
--- a/drivers/scsi/lpfc/lpfc_init.c 2015-05-29 10:19:02.0 +0200
+++ b/drivers/scsi/lpfc/lpfc_init.c 2015-05-29 10:19:56.0 +0200
@@ -4988,7 +4988,7 @@ lpfc_sli_driver_resource_setup(struct lp
/* Initialize the host templates the configured values. */
lpfc_vport_template.sg_tablesize = phba-cfg_sg_seg_cnt;
- lpfc_template.sg_tablesize = phba-cfg_sg_seg_cnt;
+ lpfc_template_s3.sg_tablesize = phba-cfg_sg_seg_cnt;
/* There are going to be 2 reserved BDEs: 1 FCP cmnd + 1 FCP rsp */
if (phba-cfg_enable_bg) {
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3] st.ko: remove unnecessary normalize_buffer
-Original Message- From: Kai Mäkisara (Kolumbus) [mailto:kai.makis...@kolumbus.fi] Sent: Monday, December 16, 2013 7:45 PM To: Strösser, Bodo Cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH 2/3] st.ko: remove unnecessary normalize_buffer On 2.12.2013, at 21.00, Bodo Stroesser bstroes...@ts.fujitsu.com wrote: From: Bodo Stroesser bstroes...@ts.fujitsu.com Date: Mon, 2 Dec 2013 18:52:10 +0100 Subject: [PATCH 2/3] st.ko: remove unnecessary normalize_buffer This patch removes an unnecessary call to normalize_buffer() in enlarge_buffer() In st_open() always a buffer of one page is allocated. When the buffer needs to be enlarged later, it does not make sense to free this page unconditionally. The original reason for this was to make the function to allocate the minimum number of segments for maximum efficiency. In some cases it was essential not to waste one segment because that would have made the allocation fail. The method of enlarge_buffer now seems to have changed. It tries to find out the minimum size of a segment that can be used with the given sg list length. I assume, this method is used to be able to allocate very big buffers, while in case of normal buffer still small chunks of memory can be used, which under memory pressure are more likely to be granted. Now there is another reason to have this “optimization”. Reading the code you probably have noticed that nowadays all segments must have the same size. If the first segment is only one page, the first allocation may it may not be possible to allocate the buffer using single page segments. This leads to freeing the pages and trying again. This is not efficient. OTOH, if the size of the currently allocated segments is too small for the buffer that needs to be allocated now, enlarge_buffer will free all current segments and do a completely new allocation using bigger segments anyway. The lines, that the patch would remove, currently free one segment, if it is the smallest possible and the only one, even if the same segment size will be allocated again a few lines below. So I'd still call it a not optimal handling of a special case, that not needs to be handled specially. Thanks, Bodo So, I think this fragment of code should not be removed. Thanks, Kai Cc: Kai Makisara kai.makis...@kolumbus.fi Signed-off-by: Bodo Stroesser bstroes...@ts.fujitsu.com --- --- a/drivers/scsi/st.c 2013-12-02 18:52:10.0 +0100 +++ b/drivers/scsi/st.c 2013-12-02 18:52:10.0 +0100 @@ -3725,9 +3725,6 @@ static int enlarge_buffer(struct st_buff if (new_size = STbuffer-buffer_size) return 1; - if (STbuffer-buffer_size = PAGE_SIZE) - normalize_buffer(STbuffer); /* Avoid extra segment */ - max_segs = STbuffer-use_sg; priority = GFP_KERNEL | __GFP_NOWARN; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] st.ko: change enlarge_buffer result
-Original Message- From: Kai Mäkisara (Kolumbus) [mailto:kai.makis...@kolumbus.fi] Sent: Monday, December 16, 2013 7:49 PM To: Strösser, Bodo Cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH 3/3] st.ko: change enlarge_buffer result On 2.12.2013, at 21.00, Bodo Stroesser bstroes...@ts.fujitsu.com wrote: From: Bodo Stroesser bstroes...@ts.fujitsu.com Date: Mon, 2 Dec 2013 18:52:10 +0100 Subject: [PATCH 3/3] st.ko: change enlarge_buffer result enlarge_buffer() just returns 1 or 0 if it could or could not allocate the requested buffer. In case of result 0, the callers always set the error to EOVERFLOW. I think, this is not a good errno for those cases, where enlarge_buffer() could not allocate the pages it needed. So I changed enlarge_buffer() to return a meaningful result (-ENOMEM or -EOVERFLOW in case of error, 0 in case of success) and the callers to use this result. ENOMEM is used for telling the user that, in variable block mode, the byte count in read() is smaller than the next block. This may not sound like proper use of this code but this is how the tape drivers have done. Oops, sorry, I missed that. So I agree fully, that the patch does not make sense. Thank you, Bodo When ENOMEM is not used, the patch would be only cosmetic. Thanks, Kai -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
