linux-kernel - Re: [PATCH v3 3/5] st: get rid of scsi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4FFDC938.4050606@suse.com>
Date:	Wed, 11 Jul 2012 11:43:04 -0700
From:	Lee Duncan <lduncan@...e.com>
To:	Kai Makisara <Kai.Makisara@...umbus.fi>
CC:	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	jeffm@...e.com
Subject: Re: [PATCH v3 3/5] st: get rid of scsi_tapes array

Kai:

Your added patch looks great, and I see you fixed the documentation as
well. Thanks for your help.


On 07/02/2012 05:16 PM, Lee Duncan wrote:
> 
> On 07/01/2012 01:57 AM, Kai Makisara wrote:
>> On Mon, 21 May 2012, Lee Duncan wrote:
>>
>>> From: Jeff Mahoney <jeffm@...e.com>
>>>
>>> st currently allocates an array to store pointers to all of the
>>> scsi_tape objects. It's used to discover available indexes to use as the
>>> base for the minor number selection and later to look up scsi_tape
>>> devices for character devices.
>>>
>>> We switch to using an IDR for minor selection and a pointer from
>>> st_modedef back to scsi_tape for the lookups.
>>>
>>> Signed-off-by: Jeff Mahoney <jeffm@...e.com>
>>> Signed-off-by: Lee Duncan <lduncan@...e.com>
>>> ---
>>>  drivers/scsi/st.c |  172 ++++++++++++++++++++---------------------------------
>>>  drivers/scsi/st.h |    2 +
>>>  2 files changed, 65 insertions(+), 109 deletions(-)
>>>
>> ... patch removed
>>
>> I have finally had time to review and test this patch set. I am sorry this 
>> has taken so long.
>>
>> I have found one change of behaviour and a theoretical problem:
>> The new code does not re-use the tape numbers when freed and re-scanned. 
>> The current code does re-use the freed numbers. Are there any reasons for 
>> this changed behaviour? (The theoretical problem is that the new code 
>> frees the tape structure but leaves the pointer in the idr tree.)
>>
>> The patch at the end of this message (applies after the whole series) is 
>> an attempt to implement re-use of tape numbers. I am not completely sure 
>> that the change is correctly placed but it seems to work.
> 
> 
> Thanks for the review, and good catch. I'll look over your added patch
> and give feedback as soon as I can.
> 
>>
>> Another minor thing is that the documentation should be updated :-)
> 
> Of course.
> 
>>
>> The patch at the end also updates the version code. I am not sure if the 
>> version code is useful, but it should be either updated or removed.
>>
>> Otherwise no problems found. I am ready to ack the patch set after the 
>> re-use thing has been resolved (one way or another).
>>
>> Thanks,
>> Kai
>>

- Lee

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/