Re: Issues with exclusive-lock code on testing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sorry, I should have looked more closely at this. I forgot that the data comes inside the message when opcode == CEPH_WATCH_EVENT_NOTIFY and a data item when opcode == CEPH_WATCH_EVENT_NOTIFY_COMPLETE. Yes, the data item isn’t used by rbd.

>> alloc_msg will have to allocate and the data item anyway, because CEPH_MSG_WATCH_NOTIFY might contain one. We only know that krbd is not expecting one iff the notify opcode is CEPH_WATCH_EVENT_NOTIFY_COMPLETE, but we do not decode that until later.
> 
> Who is going to be the user in the !NOTIFY_COMPLETE case?
> 
>> 
>> We could do as you suggest in your patch and free the data item there. It should be empty anyway for these cases, so this will not save memory. We could shrink the ceph_osd_event struct by a pointer and a size_t, though.
> 
> If you are saying we can drop notify_data and notify_data_len from the
> struct, how would you propagate the data in the !NOTIFY_COMPLETE case?
> 
> Right now the whole thing is unused and I don't see any kind of switch
> on the notify opcode in the code:
> 
>    alloc_msg():
>        if type == CEPH_MSG_WATCH_NOTIFY:
>            if data_len > 0:
>                alloc page vector, create a data item, add it to the msg
> 
>    handle_watch_notify():
>        pages = data_item->pages;
>        len = data_item->length;
>        pass pages and len into __do_event()
> 
>    __do_event():
>        assign pages and len to notify.notify_data and notify.notify_data_len,
>        both of which are unused
> 
>    destroy the message
> 
> Thanks,
> 
>                Ilya

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux