Allow EA reservation holders to read from device.
Subsystems (2)
PATCH 1/1 Allow EA reservation holders to read from device.
From: Lee Duncan <[email protected]>
For PGR reservation type Exclusive Access, allow all registrants to read from the device.
--- drivers/target/target_core_pr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/target/target_core_pr.c +12 -0
@@ -551,6 +551,18 @@ static int core_scsi3_pr_seq_non_holder(return 0 ; } + }else if (we && registered_nexus) { +/* +* Reads are allowed for Write Exclusive locks +* from all registrants. +*/ +if (cmd->data_direction ==DMA_FROM_DEVICE ) { +pr_debug ("Allowing READ CDB: 0x%02x for %s" +" reservation\n" , cdb[0 ], +core_scsi3_pr_dump_type (pr_reg_type)); + +return 0 ; + } } pr_debug("%s Conflict for %sregistered nexus %s CDB: 0x%2x" " for %s reservation\n", transport_dump_cmd_direction(cmd),
Signature · 1 line
1.8.5.2
Hi Lee,
Apologies for the delayed response, just catching up from the holidays.
Comments below.
Quoted 24 lines · Lee Duncan
From: Lee Duncan <[email protected]>
For PGR reservation type Exclusive Access, allow all registrants to read from the device.
Signed-off-by Lee Duncan <[email protected]>Cc Nicholas Bellinger <[email protected]>--- drivers/target/target_core_pr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/target/target_core_pr.c +5
-0@@ -551,6 +551,18 @@ static int core_scsi3_pr_seq_non_holder(return 0 ; } + }else if (we && registered_nexus) { +/* +* Reads are allowed for Write Exclusive locks +* from all registrants. +*/
drivers/target/target_core_pr.c static int core_scsi3_pr_seq_non_holder( · line 568
Quoted 10 lines
drivers/target/target_core_pr.c +7
-0+if (cmd->data_direction ==DMA_FROM_DEVICE ) { +pr_debug ("Allowing READ CDB: 0x%02x for %s" +" reservation\n" , cdb[0 ], +core_scsi3_pr_dump_type (pr_reg_type)); + +return 0 ; + } } pr_debug("%s Conflict for %sregistered nexus %s CDB: 0x%2x" " for %s reservation\n", transport_dump_cmd_direction(cmd),
I'm confused as to why this is necessary..
Doesn't the conditional check directly above this one ensure that all_reg=true && registered_nexus=true implicitly allow READ CDBs to be processed..?
How is this new check for PR_TYPE_WRITE_EXCLUSIVE_ALLREG reservation any different..?
--nab
Quoted 3 lines · Nicholas A. Bellinger
Hi Lee,
Apologies for the delayed response, just catching up from the holidays.
No worries.
Quoted 41 lines
Comments below.
Quoted 24 lines · Lee Duncan
From: Lee Duncan <[email protected]>
For PGR reservation type Exclusive Access, allow all registrants to read from the device.
Signed-off-by Lee Duncan <[email protected]>Cc Nicholas Bellinger <[email protected]>--- drivers/target/target_core_pr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/target/target_core_pr.c +5
-0@@ -551,6 +551,18 @@ static int core_scsi3_pr_seq_non_holder(return 0 ; } + }else if (we && registered_nexus) { +/* +* Reads are allowed for Write Exclusive locks +* from all registrants. +*/ drivers/target/target_core_pr.c static int core_scsi3_pr_seq_non_holder( · line 568
Quoted 10 lines
drivers/target/target_core_pr.c +7
-0+if (cmd->data_direction ==DMA_FROM_DEVICE ) { +pr_debug ("Allowing READ CDB: 0x%02x for %s" +" reservation\n" , cdb[0 ], +core_scsi3_pr_dump_type (pr_reg_type)); + +return 0 ; + } } pr_debug("%s Conflict for %sregistered nexus %s CDB: 0x%2x" " for %s reservation\n", transport_dump_cmd_direction(cmd),I'm confused as to why this is necessary..
Quoted 3 lines
Doesn't the conditional check directly above this one ensure that all_reg=true && registered_nexus=true implicitly allow READ CDBs to be processed..?
No, the section right above it handles "All Registrants" or "Registrants Only" reservations, but not "normal" reservations.
In other words, if using "sg_persist" to create the registration, the problem occurs for type 1, i.e. regular old "write exclusive".
And the section above it is allowing reads and writes, and we only want to allow READs for the non-reservation holders in this case.
Quoted 3 lines
How is this new check for
PR_TYPE_WRITE_EXCLUSIVE_ALLREGreservation any different..?
Er, I believe they are totally different. That patch was about allowing re-registration. This one is about allowing non-reservation-holding registrants to read from the device when the reservation is of type Write Exclusive. Can we agree that this should be allowed?
Quoted 3 lines
--nab
Signature · 2 lines
Lee Duncan SUSE Labs
Quoted 52 lines · Lee Duncan
Quoted 38 lines · Nicholas A. Bellinger
Quoted 24 lines · Lee Duncan
From: Lee Duncan <[email protected]>
For PGR reservation type Exclusive Access, allow all registrants to read from the device.
Signed-off-by Lee Duncan <[email protected]>Cc Nicholas Bellinger <[email protected]>--- drivers/target/target_core_pr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/target/target_core_pr.c +5
-0@@ -551,6 +551,18 @@ static int core_scsi3_pr_seq_non_holder(return 0 ; } + }else if (we && registered_nexus) { +/* +* Reads are allowed for Write Exclusive locks +* from all registrants. +*/ drivers/target/target_core_pr.c static int core_scsi3_pr_seq_non_holder( · line 568
Quoted 10 lines
drivers/target/target_core_pr.c +7
-0+if (cmd->data_direction ==DMA_FROM_DEVICE ) { +pr_debug ("Allowing READ CDB: 0x%02x for %s" +" reservation\n" , cdb[0 ], +core_scsi3_pr_dump_type (pr_reg_type)); + +return 0 ; + } } pr_debug("%s Conflict for %sregistered nexus %s CDB: 0x%2x" " for %s reservation\n", transport_dump_cmd_direction(cmd),I'm confused as to why this is necessary..
Quoted 3 lines
Doesn't the conditional check directly above this one ensure that all_reg=true && registered_nexus=true implicitly allow READ CDBs to be processed..?
No, the section right above it handles "All Registrants" or "Registrants Only" reservations, but not "normal" reservations.
In other words, if using "
sg_persist" to create the registration, the problem occurs for type 1, i.e. regular old "write exclusive".And the section above it is allowing reads and writes, and we only want to allow READs for the non-reservation holders in this case.
Quoted 1 line
Quoted 8 lines
Quoted 2 lines
How is this new check for
PR_TYPE_WRITE_EXCLUSIVE_ALLREGreservation any different..?Er, I believe they are totally different. That patch was about allowing re-registration. This one is about allowing non-reservation-holding registrants to read from the device when the reservation is of type Write Exclusive. Can we agree that this should be allowed?
Ah, I was getting confused by the 'all registrants' part of the commit message.
Applying to target-pending/master now with a more explicit commit message.
Thanks Lee!
--nab