> -----Original Message----- > From: "Kai Mäkisara (Kolumbus)" [mailto:kai.makisara@xxxxxxxxxxx] > Sent: Monday, December 16, 2013 7:45 PM > To: Strösser, Bodo > Cc: linux-scsi@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/3] st.ko: remove unnecessary normalize_buffer > > > On 2.12.2013, at 21.00, Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > wrote: > > > From: Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > > 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.makisara@xxxxxxxxxxx> > > Signed-off-by: Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > > > > --- > > > > --- a/drivers/scsi/st.c 2013-12-02 18:52:10.000000000 +0100 > > +++ b/drivers/scsi/st.c 2013-12-02 18:52:10.000000000 +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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
