Re: [PATCH 15/15] nvmet-rdma: add metadata/T10-PI support

[Max Gurtovoy <maxg@mellanox.com>]

On 5/1/2020 7:48 PM, Christoph Hellwig wrote:
>> +static u16 nvmet_rdma_check_pi_status(struct ib_mr *sig_mr)
>> +{
>> +	struct ib_mr_status mr_status;
>> +	int ret;
>> +	u16 status = 0;
>> +
>> +	if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
>> +		return 0;
> I'd expect this in the callers..
>
>> +static void nvmet_rdma_set_sig_domain(struct blk_integrity *bi,
>> +		struct nvme_command *cmd, struct ib_sig_domain *domain,
>> +		u16 control)
>> +{
>> +	domain->sig_type = IB_SIG_TYPE_T10_DIF;
>> +	domain->sig.dif.bg_type = IB_T10DIF_CRC;
>> +#ifdef CONFIG_BLK_DEV_INTEGRITY
>> +	domain->sig.dif.pi_interval = 1 << bi->interval_exp;
>> +#endif
> With the genhd.h tweak I sent in response to the host code, we
> shouldn't need this.
>
>> +	if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
>> +		return;
> Same comment as above.
>
>> +	if (nvmet_rdma_need_data_out(rsp)) {
>> +		if (rsp->req.md_len)
> Can we have a helper for metadata len that also uses
> IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) to let the compiler garbage collect
> the whole code if it is compiled out?

We have it in:

static inline u32 nvmet_rw_metadata_len(struct nvmet_req *req)
{
         if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
                 return 0;
         return ((u32)le16_to_cpu(req->cmd->rw.length) + 1) *
                         req->ns->metadata_size;
}


and

req->metadata_len = nvmet_rw_metadata_len(req);

is it good enough for the compiler ?



_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme