Re: [RFC 4/7] zuf: zuf-rootfs && zuf-core

[Boaz Harrosh <boazh@netapp.com>]

On 14/03/18 14:56, Nikolay Borisov wrote:
> 
> 
> On 13.03.2018 19:36, Boaz Harrosh wrote:
>>
>> zuf-core established the communication channels with
>> the zus UM Server.
>>
>> zuf-root is a psuedo FS that the zus communicates through,
>> registers new file-systems. receives new mount requests.
>>
>> In this patch we have the bring up of that special FS, and
>> the core communication mechanics. Which is the Novelty
>> of this code submission.
>>
>> The zuf-rootfs (-t zuf) is usually by default mounted on
>> /sys/fs/zuf. If an admin wants to run more server applications
>> (Note that each server application supports many types of FSs)
>> He/she can mount a second instance of -t zuf and point the new
>> Server to it.
>>
>> (Otherwise a second instance attempting to communicate with a
>>  busy zuf will fail)
>>
>> TODO: How to trigger a first mount on module_load. Currently
>> admin needs to manually "mount -t zuf none /sys/fs/zuf"
>>
>> Signed-off-by: Boaz Harrosh <boazh@netapp.com>
> <snip>
> 
>> +	while (!relay_is_fss_waiting(&zt->relay)) {
>> +		mb();
> Not something for this early in the development cycle but something to
> keep in mind:
> 
> Always document all assumptions re. memory barriers usage + intended
> pairing scenario otherwise it's very hard to reason whether this is
> correct or not. In fact barriers without comments are considered broken.
> 

Yes. I totally agree. I love it how you commented on the ugliest piece
of code in all of this.

This is BTW totally very wrong and the mb is wrong and is a bandade
over the wrong kind of hurt. Because in theory coming out of sleep
I might be scheduled on another core and I should then pick up a new
zt instead of syncing with the now wrong one.

for a POC it was fine, I get here maybe 0.17% of the times but I will
totally need to put some thought and love into this problem.

I kind of want to move from that Relay object to what Binder is using
and hope to shave off some more latency. So I'll see if I will fix
this or move to new code

Thank you for looking, Yes this is a bad contraption
Boaz

>> +		if (unlikely(!zt->file))
>> +			return -EIO;
>> +		zuf_dbg_err("[%d] can this be\n", cpu);
>> +		/* FIXME: Do something much smarter */
>> +		msleep(10);
>> +		mb();
>> +	}
>> +
>> +	zt->next_opt = hdr;
>> +	zt->pages = pages;
>> +	zt->nump = nump;
>> +
>> +	relay_fss_wakeup_app_wait(&zt->relay, NULL);
>> +
>> +	return zt->file ? hdr->err : -EIO;
>> +}
> 
> <snip>
>