lkml.org 
[lkml]   [2014]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/2] arcmsr: simplify ioctl data read/write
On 09/24/2014 11:33 AM, Ching Huang wrote:
> From: Ching Huang <ching2048@areca.com.tw>
>
> This patch is relative to:
> http://git.infradead.org/users/hch/scsi-queue.git/tree/drivers-for-3.18:/drivers/scsi/arcmsr
>
> change in v5:
> 1. rename firstindex to getIndex, lastindex to putIndex for readability
> 2. define ARCMSR_API_DATA_BUFLEN as 1032
> 3. simplify ioctl data read by marcro CIRC_CNT_TO_END and CIRC_CNT
>
> Signed-off-by: Ching Huang <ching 2048@areca.com.tw>
> ---
>
...
> + pQbuffer = &acb->wqbuffer[acb->wqbuf_putIndex];
> + cnt2end = ARCMSR_MAX_QBUFFER - acb->wqbuf_putIndex;
> + if (user_len > cnt2end) {
> + memcpy(pQbuffer, ptmpuserbuffer, cnt2end);
> + ptmpuserbuffer += cnt2end;
> + user_len -= cnt2end;
> + acb->wqbuf_putIndex = 0;
> + pQbuffer = acb->wqbuffer;
> }
> + memcpy(pQbuffer, ptmpuserbuffer, user_len);
> + acb->wqbuf_putIndex += user_len;
> + acb->wqbuf_putIndex %= ARCMSR_MAX_QBUFFER;
> + if (acb->acb_flags & ACB_F_MESSAGE_WQBUFFER_CLEARED) {
This test^ is most likely useless, it looks like you set the
ACB_F_MESSAGE_WQBUFFER_CLEARED every time you have added some data to the buffer
and clear it when the buffer gets empty. I think you could get rid of
the ACB_F_MESSAGE_WQBUFFER_CLEARED completely. Also the ACB_F_MESSAGE_RQBUFFER_CLEARED doesn't
seems to be ever evaluated.
I'm not sure with the ACB_F_MESSAGE_WQBUFFER_READED, but that one probably is also
a candidate for removal.
...
> @@ -678,15 +679,15 @@ struct AdapterControlBlock
> unsigned int uncache_size;
> uint8_t rqbuffer[ARCMSR_MAX_QBUFFER];
> /* data collection buffer for read from 80331 */
> - int32_t rqbuf_firstindex;
> + int32_t rqbuf_getIndex;
What is the reason for using an exact size int32 (instead of a plain int) here?
> /* first of read buffer */
> - int32_t rqbuf_lastindex;
> + int32_t rqbuf_putIndex;
> /* last of read buffer */
> uint8_t wqbuffer[ARCMSR_MAX_QBUFFER];
> /* data collection buffer for write to 80331 */
> - int32_t wqbuf_firstindex;
> + int32_t wqbuf_getIndex;
> /* first of write buffer */
> - int32_t wqbuf_lastindex;
> + int32_t wqbuf_putIndex;
> /* last of write buffer */
> uint8_t devstate[ARCMSR_MAX_TARGETID][ARCMSR_MAX_TARGETLUN];
> /* id0 ..... id15, lun0...lun7 */

The comments I've added are not directly related to this patch,
but you may still address them in a new patch
so -
Reviewed-by: Tomas Henzl <thenzl@redhat.com>





\
 
 \ /
  Last update: 2014-09-24 18:21    [W:0.047 / U:2.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site