Re: [PATCH 0/2] scsi: target: tcmu: Replace IDR and radix_tree with XArray

[Bodo Stroesser <bostroesser@gmail.com>]

On 26.02.21 04:59, michael.christie@oracle.com wrote:
> On 2/24/21 12:53 PM, Bodo Stroesser wrote:
>> This small series is based on Martin's for-next.
>>
>> Future patches will need something like the - meanwhile removed -
>> radix_tree_for_each_contig macro.
>> Since general direction is to use xarray as replacement for
>> radix_tree and IDR, instead of re-introducing or open coding the
>> removed macro, with this series we switch over to xarray API.
>> Based on xarray a future patch easily can implement an analog
>> to radix_tree_for_each_contig.
>>
>> Bodo Stroesser (2):
>>    scsi: target: tcmu: Replace IDR by XArray
>>    scsi: target: tcmu: Replace radix_tree with XArray
>>
>>   drivers/target/target_core_user.c | 64 +++++++++++++++++++--------------------
>>   1 file changed, 31 insertions(+), 33 deletions(-)
>>
> 
> Looks ok to me.
> 
> Reviewed-by: Mike Christie <michael.christie@oracle.com>
> 
> I think in a separate patch we need to change:
> 
> +		if (xa_store(&udev->data_blocks, dbi, page, GFP_KERNEL))
>   			goto err_insert;
> 
> to GFP_NOIO. There were some users doing tcm loop  + tcmu and
> were hitting issues when GFP_KERNEL lead to write backs back on
> to the same tcmu device. We tried to change all the gfp flags but
> we missed that one, because it was hidden in the radix tree's
> xa_flags.
> 

But then, for consistency reasons, shouldn't we change

+	if (xa_alloc(&udev->commands, &cmd_id, tcmu_cmd, XA_LIMIT(1, 0xffff),
+		     GFP_NOWAIT) < 0) {

to GFP_NOIO also?

Additionally I have to change memory allocation in tcmu_tmr_notify from
GFP_KERNEL to GFP_NOIO, since it happens while holding cmdr_lock mutex,
which could also cause the problems you desribed.

Shouldn't we better change to new API memalloc_noio_save and
memalloc_noio_restore in that new patch?