2015-04-26 18:44 GMT+09:00 Sagi Grimberg <sagig@xxxxxxxxxxxxxxxxxx>:
>>> @@ -2181,6 +2182,12 @@ static inline void
>>> transport_reset_sgl_orig(struct se_cmd *cmd)
>>>
>>> static inline void transport_free_pages(struct se_cmd *cmd)
>>> {
>>> + if (!(cmd->se_cmd_flags & SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC)) {
>>> + transport_free_sgl(cmd->t_prot_sg, cmd->t_prot_nents);
>>> + cmd->t_prot_sg = NULL;
>>> + cmd->t_prot_nents = 0;
>>> + }
>>> +
>>
>>
>> Hi Akinobu,
>>
>> Any reason why this changed it's location to the start of the function?
Because when SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC is set, it will not
reach the tail of the function. So when SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC
is cleared and SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC is set,
se_cmd->t_prot_sg leaks.
>>> if (cmd->se_cmd_flags & SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC) {
>>> /*
>>> * Release special case READ buffer payload required for
>>> @@ -2204,10 +2211,6 @@ static inline void transport_free_pages(struct
>>> se_cmd *cmd)
>>> transport_free_sgl(cmd->t_bidi_data_sg, cmd->t_bidi_data_nents);
>>> cmd->t_bidi_data_sg = NULL;
>>> cmd->t_bidi_data_nents = 0;
>>> -
>>> - transport_free_sgl(cmd->t_prot_sg, cmd->t_prot_nents);
>>> - cmd->t_prot_sg = NULL;
>>> - cmd->t_prot_nents = 0;
>>> }
>>>
>>> /**
>>> @@ -2346,6 +2349,17 @@ transport_generic_new_cmd(struct se_cmd *cmd)
>>> int ret = 0;
>>> bool zero_flag = !(cmd->se_cmd_flags & SCF_SCSI_DATA_CDB);
>>>
>>> + if (!(cmd->se_cmd_flags & SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC)) {
>>> + if (cmd->prot_op != TARGET_PROT_NORMAL) {
>>
>>
>> This seems wrong,
>>
>> What will happen for transports that will actually to allocate
>> protection SGLs? The allocation is unreachable since
>> SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC is not set...
>
>
> Umm, actually this is reachable... But I still think the condition
> should be the other way around (saving a condition in some common
> cases).
Do you mean you prefer below?
if (cmd->prot_op != TARGET_PROT_NORMAL &&
!(cmd->se_cmd_flags & SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC)) {
...
>>
>> I'd say this needs to be:
>>
>> if (cmd->prot_op != TARGET_PROT_NORMAL &&
>> !(cmd->se_cmd_flags & SCF_PASSTHROUGH_PROT_SG_TO_MEM_NOALLOC)) {
--
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
