On 4/27/2015 3:57 PM, Akinobu Mita wrote:
2015-04-26 18:44 GMT+09:00 Sagi Grimberg <sa...@dev.mellanox.co.il>:
@@ -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.
I see. That's fine...
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 think it will be better.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
