-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
