public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Stefano Manni <stefano.manni@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Oleg Drokin <oleg.drokin@intel.com>,
	Andreas Dilger <andreas.dilger@intel.com>,
	James Simmons <jsimmons@infradead.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	lustre-devel@lists.lustre.org
Subject: Re: [PATCH 1/4] staging: lustre: fixed signedness of some socklnd params
Date: Fri, 24 Nov 2017 00:47:21 +0100	[thread overview]
Message-ID: <1511480841.8829.3.camel@gmail.com> (raw)
In-Reply-To: <20171123121306.nu66ktiqjedongpp@mwanda>

On Thu, 2017-11-23 at 15:13 +0300, Dan Carpenter wrote:
> I've looked through this series and I feel like none of these are
> real
> bugs.  It's just about type safety and being consistent.  Which are
> good
> things.  I'm not sure that I like the parts where we make the
> variables
> signed.
> 
> Here "nscheds" is the number of threads.  How can we have a negative
> number?  I think it should be unsigned.  It's way more tricky to
> change
> the rest of the code, and leave nscheds unsigned int but I think it's
> probably the right thing.
> 
> regards,
> dan carpenter
> 

I've made the module param nsched signed because the
ksock_tunables.ksnd_nscheds (the real container) is signed too.

I definitely agree with you that it does not make sense to have a
negative number of threads.
In my opinion it's better to fix this inconsistency between the param
and the container and then try submit another patch to harmonize
signedness around the code.

Thanks,
Stefano

  reply	other threads:[~2017-11-23 23:47 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22 19:38 [PATCH 0/4] staging: lustre: fixed some signedness warns from sparse Stefano Manni
2017-11-22 19:38 ` [PATCH 1/4] staging: lustre: fixed signedness of some socklnd params Stefano Manni
2017-11-23 12:13   ` Dan Carpenter
2017-11-23 23:47     ` Stefano Manni [this message]
2017-11-24 12:49       ` Dan Carpenter
2017-11-22 19:38 ` [PATCH 2/4] staging: lustre: fixed signedness of llite Stefano Manni
2017-11-22 19:38 ` [PATCH 3/4] staging: lustre: fixed signedness of lov Stefano Manni
2017-11-24 15:42   ` Greg Kroah-Hartman
2017-11-22 19:38 ` [PATCH 4/4] staging: lustre: fixed signedness of obdclass Stefano Manni
2017-11-23  4:59 ` [PATCH 0/4] staging: lustre: fixed some signedness warns from sparse Tobin C. Harding
2017-11-23 11:51   ` Dan Carpenter
2017-11-23 22:20     ` Tobin C. Harding
2017-11-23 23:32       ` Stefano Manni
2017-11-24 13:02       ` Dan Carpenter
2017-11-24 21:37         ` Tobin C. Harding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1511480841.8829.3.camel@gmail.com \
    --to=stefano.manni@gmail.com \
    --cc=andreas.dilger@intel.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jsimmons@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=oleg.drokin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox