Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <67726654-2c13-2c41-c002-30dc99db8497@samsung.com>
Date:   Mon, 5 Sep 2022 14:57:08 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
        Thunderbird/91.11.0
Subject: Re: [PATCH v12 13/13] dm: add power-of-2 target for zoned devices
 with non power-of-2 zone sizes
Content-Language: en-US
To:     Mike Snitzer <snitzer@redhat.com>
CC:     <agk@redhat.com>, <snitzer@kernel.org>, <axboe@kernel.dk>,
        <damien.lemoal@opensource.wdc.com>, <hch@lst.de>,
        Damien Le Moal <damien.lemoal@wdc.com>, <bvanassche@acm.org>,
        <pankydev8@gmail.com>,
        Johannes Thumshirn <johannes.thumshirn@wdc.com>,
        <linux-kernel@vger.kernel.org>, <linux-nvme@lists.infradead.org>,
        <linux-block@vger.kernel.org>, <dm-devel@redhat.com>,
        <gost.dev@samsung.com>, <jaegeuk@kernel.org>,
        <matias.bjorling@wdc.com>
From:   Pankaj Raghav <p.raghav@samsung.com>
In-Reply-To: <YxJwd7xcZRdznsYz@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
CMS-TYPE: 201P
References: <20220823121859.163903-1-p.raghav@samsung.com>
        <CGME20220823121915eucas1p10296d6a2ab00de5e73791a7d6b8e39de@eucas1p1.samsung.com>
        <20220823121859.163903-14-p.raghav@samsung.com>
        <YxJttvB2Z5I58SQ5@redhat.com> <YxJwd7xcZRdznsYz@redhat.com>
Precedence: bulk

>>
>> 1) why is a partial mapping of the underlying device disallowed?
>> 2) why is it assumed all IO is read-only? (talk to me and others like
>>    we don't know the inherent limitations of this class of zoned hw)
>>
>> On a code level:
>> 1) are you certain you're properly failing all writes?
>>    - are writes allowed to the "zone capacity area" but _not_
>>      allowed to the "emulated zone area"? (if yes, _please document_). 
>> 2) yes, you absolutely need to implement the .status target_type hook
>>    (for both STATUS and TABLE).
>> 3) really not loving the nested return (of DM_MAPIO_SUBMITTED or
>>    DM_MAPIO_REMAPPED) from methods called from dm_po2z_map().  Would
>>    prefer to not have to do a depth-first search to see where and when
>>    dm_po2z_map() returns a DM_MAPIO_XXX unless there is a solid
>>    justification for it.  To me it just obfuscates the DM interface a
>>    bit too much. 
>>
>> Otherwise, pretty clean code and nothing weird going on.
>>
>> I look forward to seeing your next (final?) revision of this patchset.
> 
> Thinking further.. I'm left confused about just what the heck this
> target is assuming.
> 
> E.g.: feels like its exposing a readonly end of the zone is very
> bi-polar... yet no hint to upper layer it shouldn't write to that
> read-only end (the "emulated zone").. but there has to be some zoned
> magic assumed?  And I'm just naive?
> 

You are absolutely right about "zoned magic". Applications that use a zoned
block device are aware of the zone capacity and zone size. BLKREPORTZONE
ioctl is typically used to get the zone information from a zoned block device.

This target adjusts the zone report so that zone size and zone capacity are
modified correctly (see dm_po2z_report_zones() and
dm_po2z_report_zones_cb() functions).