[pnfs] [PATCH 10/10] SQUASHME: objlayout: osd_layoutupdate wire-format to latest draft

[Benny Halevy]

On Aug. 11, 2009, 18:10 +0300, Boaz Harrosh <bharrosh at panasas.com> wrote:
> On 08/11/2009 04:34 PM, Benny Halevy wrote:
>> On Aug. 10, 2009, 21:21 +0300, Boaz Harrosh <bharrosh at panasas.com> wrote:
>>> * define new xdr encode/decode layoutupdate helpers
>>> * use above in encoding of osd_layoutupdate buffer in
>>>   objlayout_setup_layoutcommit()
>>>
>>> [Note: This will break pan_fs pnfsd server part, as it is still
>>>  using the old pnfs_osd_layoutupdate4 wire-format]
>>>
>>> Signed-off-by: Boaz Harrosh <bharrosh at panasas.com>
>>> ---
>>>  fs/nfs/objlayout/objlayout.c        |   23 +++++++++++++----------
>>>  fs/nfs/objlayout/pnfs_osd_xdr.c     |   19 +++++++++++++++++++
>>>  fs/nfs/objlayout/pnfs_osd_xdr.h     |    9 +++++++++
>>>  fs/nfs/objlayout/pnfs_osd_xdr_srv.c |   17 +++++++++++++++++
>>>  4 files changed, 58 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
>>> index ff9c8e4..6f709c3 100644
>>> --- a/fs/nfs/objlayout/objlayout.c
>>> +++ b/fs/nfs/objlayout/objlayout.c
>>> @@ -470,29 +470,32 @@ objlayout_setup_layoutcommit(struct pnfs_layout_type *pnfslay,
>>>  {
>>>  	struct pnfs_layoutcommit_arg *arg = &data->args;
>>>  	struct objlayout *objlay;
>>> +	struct pnfs_osd_layoutupdate lou;
>>>  	int status = 0;
>>> -	s64 delta;
>>> -	s64 *buf;
>>> +	u32 *buf, *p;
>>>  
>>>  	dprintk("%s: Begin\n", __func__);
>>>  
>>>  	objlay = PNFS_LD_DATA(pnfslay);
>>>  
>>> -	delta = objlayout_atomic64_xchg(&objlay->delta_space_used, 0);
>>> -
>>> -	if (!delta)
>>> -		goto out;
>>> +	lou.dsu_delta = objlayout_atomic64_xchg(&objlay->delta_space_used, 0);
>>> +	lou.dsu_valid = (lou.dsu_delta != 0);
>> wrong...
>> !dsu_valid is very different from (dsu_valid && !dsu_delta)
>> In the former case, the client says it doesn't know the delta;
>> in the latter, it knows it and it's zero.
>>
>> From the server perspective, if the client sent dsu_valid == 0
>> it must go and fetch the used capacity attribute from storage,
>> otherwise it can use the client provided value and accumulate it
>> locally.
>>
> 
> I'm just doing exactly what is done today, converted to new wire-format
> 
> It used to be opaque-empty means not available switched on if (!delta)
> Now it is dsu_valid = (!delta) plus that olu_ioerr_flag.
> 
> To do what you say is to change the API with IO engine which is for a new
> patch.

OK. but this must be fixed.

> 
>> Benny
>>
>>> +	spin_lock(&objlay->lock);
>>> +	lou.olu_ioerr_flag = list_empty(&objlay->err_list);

Hmm, shouldn't the err flag be set to true if the
err_list is NOT empty?

>>> +	spin_unlock(&objlay->lock);
>>>  
> 
> Benny if I already have a spin-lock per-layout, and as stated above
> I want an API to increment the delta - so valid flag can be set as well.
> Should I drop the atomic and use the objlay->lock?

Yeah, if we already grab the spin lock we might as well do the
delta exchange under it.

We should probably add a delta_space_valid flag to struct
objlayout_io_state so if the OSD does not support it we can
propagate that into the layout and from this point keep it
as invalid, i.e. in objlayout_write_done:

	if (!state->delta_space_valid)
		objlay->delta_space_valid = false;
	else if (objlay->delta_space_valid)
		objlay->delta_space_used += objlay->delta_space_valid

objlay->delta_space_valid can be initialized to true
so we pass objlay->delta_space_used == 0 if it was never updated.

Benny

> 
> Boaz
>