st.ko: change enlarge_buffer result
Subsystems (2)
PATCH 3/3 st.ko: change enlarge_buffer result
From: Bodo Stroesser <[email protected]>
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.
I also removed a check in setup_buffering() that is done in enlarge_buffer() anyway.
---
drivers/scsi/st.c +6 -9
@@ -1221,10 +1221,9 @@ static int st_open(struct inode *inode, }/* See that we have at least a one page buffer available */ -if (!enlarge_buffer (STp->buffer,PAGE_SIZE , STp->restr_dma)) { +if ((retval =enlarge_buffer (STp->buffer,PAGE_SIZE , STp->restr_dma)) !=0 ) {printk (KERN_WARNING "%s: Can't allocate one page tape buffer.\n" , name); - retval = (-EOVERFLOW ); goto err_out; }
@@ -1517,11 +1516,9 @@ static int setup_buffering(struct scsi_tclear_buffer (STbp); } -if (bufsize > STbp->buffer_size && - !enlarge_buffer (STbp, bufsize, STp->restr_dma)) { +if ((retval =enlarge_buffer (STbp, bufsize, STp->restr_dma)) !=0 ) {printk (KERN_WARNING "%s: Can't allocate %d byte tape buffer.\n" ,tape_name (STp), bufsize); - retval = (-EOVERFLOW ); goto out; }if (STp->block_size)
@@ -3723,7 +3720,7 @@ static int enlarge_buffer(struct st_buffgfp_t priority;if (new_size <= STbuffer->buffer_size) -return 1 ; +return 0 ; max_segs = STbuffer->use_sg;
@@ -3747,7 +3744,7 @@ static int enlarge_buffer(struct st_buff }if (max_segs * (PAGE_SIZE << order) < new_size) {if (order ==ST_MAX_ORDER ) -return 0 ; +return -EOVERFLOW ;normalize_buffer (STbuffer);return enlarge_buffer (STbuffer, new_size, need_dma); }
@@ -3760,7 +3757,7 @@ static int enlarge_buffer(struct st_buffif (!page) {DEB (STbuffer->buffer_size = got);normalize_buffer (STbuffer); -return 0 ; +return -ENOMEM ; } STbuffer->frp_segs +=1 ;
@@ -3771,7 +3768,7 @@ static int enlarge_buffer(struct st_buff } STbuffer->b_data =page_address (STbuffer->reserved_pages[0 ]); -return 1 ; +return 0 ; }
Quoted 7 lines · Bodo Stroesser
From: Bodo Stroesser <[email protected]>
Date: Mon, 2 Dec 2013 18:52:10 +0100
Subject: [PATCH 3/3] st.ko: changeenlarge_bufferresult
enlarge_buffer()just returns 1 or 0 if it could or could not allocate the requested buffer.
Quoted 9 lines
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 (-ENOMEMor-EOVERFLOWin 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. When ENOMEM is not used, the patch would be only cosmetic.
Thanks,
Kai
Quoted 24 lines
-----Original Message-----
From: "Kai Mäkisara (Kolumbus)" [mailto:[email protected]]
Sent: Monday, December 16, 2013 7:49 PM
To: Strösser, Bodo
Cc: [email protected]
Subject: Re: [PATCH 3/3] st.ko: changeenlarge_bufferresultQuoted 12 lines · Bodo Stroesser
From: Bodo Stroesser <[email protected]>
Date: Mon, 2 Dec 2013 18:52:10 +0100
Subject: [PATCH 3/3] st.ko: changeenlarge_bufferresult
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.
Quoted 7 lines
Quoted 4 lines
So I changed
enlarge_buffer()to return a meaningful result (-ENOMEMor-EOVERFLOWin 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
Quoted 5 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html