Re: [RFC PATCH 04/17] zuf: zuf-core The ZTs

[Boaz Harrosh <openosd@gmail.com>]

On 26/02/19 20:34, Schumaker, Anna wrote:
> On Tue, 2019-02-19 at 13:51 +0200, Boaz harrosh wrote:
<>
>> zuf-core established the communication channels with the ZUS
>> User Mode Server.
>>
>> In this patch we have the core communication mechanics.
>> Which is the Novelty of this project.
>> (See previous submitted documentation for more info)
>>
>> Users will come later in the patchset
>>
<>
>> +static inline int relay_fss_wait(struct relay *relay)
>> +{
>> +       int err;
>> +
>> +       relay->fss_waiting = true;
>> +       relay->fss_wakeup = false;
>> +       err =  wait_event_interruptible(relay->fss_wq, relay->fss_wakeup);
>> +
>> +       return err;
> 
> Could you just do: "return wait_event_interruptible()" directly, instead of
> using the err variable?
> 

Totally there used to be a dbg_print here there for the reminder of that time
Will change ...

>> +}
>> +
<>
>> +static struct zufc_thread *_zt_from_cpu(struct zuf_root_info *zri,
>> +                                       int cpu, uint chan)
>> +{
>> +       return per_cpu_ptr(zri->_ztp->_all_zt[chan], cpu);
>> +}
>> +
>> +static int _zt_from_f(struct file *filp, int cpu, uint chan,
>> +                     struct zufc_thread **ztp)
>> +{
>> +       *ztp = _zt_from_cpu(ZRI(filp->f_inode->i_sb), cpu, chan);
>> +       if (unlikely(!*ztp))
>> +               return -ERANGE;
>> +       return 0;
> 
> I'm curious if there is a reason you did it this way instead of making use of
> the ERR_PTR() macro to return ztp directly?
> 

For one now looking at it I hate the name its wrong. I will change that. It is done
like that because it used to be used in many places and I did not want every place
to have its print and invent its own error code.

But now it has a single user I might just fold it into its only user.
All other places must use _zt_from_f_private. Cool I'll kill it.

>> +}
>> +
<>

Thanks, will fix
Boaz

>> +static int _zu_init(struct file *file, void *parg)
>> +{
>> +       struct zufc_thread *zt;
>> +       int cpu = smp_processor_id();
>> +       struct zufs_ioc_init zi_init;
>> +       int err;
>> +
>> +       err = copy_from_user(&zi_init, parg, sizeof(zi_init));
>> +       if (unlikely(err)) {
>> +               zuf_err("=>%d\n", err);
>> +               return err;
>> +       }
>> +       if (unlikely(zi_init.channel_no >= ZUFS_MAX_ZT_CHANNELS)) {
>> +               zuf_err("[%d] channel_no=%d\n", cpu, zi_init.channel_no);
>> +               return -EINVAL;
>> +       }
>> +
>> +       zuf_dbg_zus("[%d] aff=0x%lx channel=%d\n",
>> +                   cpu, zi_init.affinity, zi_init.channel_no);
>> +
>> +       zi_init.hdr.err = _zt_from_f(file, cpu, zi_init.channel_no, &zt);
>> +       if (unlikely(zi_init.hdr.err)) {
>> +               zuf_err("=>%d\n", err);
>> +               goto out;
>> +       }
>> +
>> +       if (unlikely(zt->hdr.file)) {
>> +               zi_init.hdr.err = -EINVAL;
>> +               zuf_err("[%d] !!! thread already set\n", cpu);
>> +               goto out;
>> +       }
>> +
>> +       relay_init(&zt->relay);
>> +       zt->hdr.type = zlfs_e_zt;
>> +       zt->hdr.file = file;
>> +       zt->no = cpu;
>> +       zt->chan = zi_init.channel_no;
>> +
>> +       zt->max_zt_command = zi_init.max_command;
>> +       zt->opt_buff = vmalloc(zi_init.max_command);
>> +       if (unlikely(!zt->opt_buff)) {
>> +               zi_init.hdr.err = -ENOMEM;
>> +               goto out;
>> +       }
>> +       _fill_buff(zt->opt_buff, zi_init.max_command / sizeof(ulong));
>> +
>> +       file->private_data = &zt->hdr;
>> +out:
>> +       err = copy_to_user(parg, &zi_init, sizeof(zi_init));
>> +       if (err)
>> +               zuf_err("=>%d\n", err);
>> +       return err;
>> +}
>> +
>> +struct zufc_thread *_zt_from_f_private(struct file *file)
>> +{
>> +       struct zuf_special_file *zsf = file->private_data;
>> +
>> +       WARN_ON(zsf->type != zlfs_e_zt);
>> +       return container_of(zsf, struct zufc_thread, hdr);
>> +}
>> +
>> +/* Caller checks that file->private_data != NULL */
>> +static void zufc_zt_release(struct file *file)
>> +{
>> +       struct zufc_thread *zt = _zt_from_f_private(file);
>> +
<>