All of lore.kernel.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Vaibhav Jain <vaibhav@linux.ibm.com>
Cc: Ira Weiny <ira.weiny@intel.com>,
	Santosh Sivaraj <santosh@fossix.org>,
	linux-nvdimm@lists.01.org,
	"Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
	linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	"Oliver O'Halloran" <oohall@gmail.com>,
	Dan Williams <dan.j.williams@intel.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v10 6/6] powerpc/papr_scm: Implement support for PAPR_PDSM_HEALTH
Date: Sat, 6 Jun 2020 13:09:10 -0500	[thread overview]
Message-ID: <20200606180910.GW31009@gate.crashing.org> (raw)
In-Reply-To: <87wo4kfk58.fsf@linux.ibm.com>

On Sat, Jun 06, 2020 at 06:04:11PM +0530, Vaibhav Jain wrote:
> >> +	/* update health struct with various flags derived from health bitmap */
> >> +	health = (struct nd_papr_pdsm_health) {
> >> +		.dimm_unarmed = p->health_bitmap & PAPR_PMEM_UNARMED_MASK,
> >> +		.dimm_bad_shutdown = p->health_bitmap & PAPR_PMEM_BAD_SHUTDOWN_MASK,
> >> +		.dimm_bad_restore = p->health_bitmap & PAPR_PMEM_BAD_RESTORE_MASK,
> >> +		.dimm_encrypted = p->health_bitmap & PAPR_PMEM_ENCRYPTED,
> >> +		.dimm_locked = p->health_bitmap & PAPR_PMEM_SCRUBBED_AND_LOCKED,
> >> +		.dimm_scrubbed = p->health_bitmap & PAPR_PMEM_SCRUBBED_AND_LOCKED,
> >
> > Are you sure these work?  These are not assignments to a bool so I don't think
> > gcc will do what you want here.
> Yeah, somehow this slipped by and didnt show up in my tests. I checked
> the assembly dump and seems GCC was silently skipping initializing these
> fields without making any noise.

It's not "skipping" that, it initialises the field to 0, just like your
code said it should :-)

If you think GCC should warn for this, please open a PR?  It is *normal*
for bit-fields to be truncated from what is assigned to it, but maybe we
could warn for it in the 1-bit case (we currently don't seem to, even
when the bit-field type is _Bool).

Thanks,


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Vaibhav Jain <vaibhav@linux.ibm.com>
Cc: Santosh Sivaraj <santosh@fossix.org>,
	linux-nvdimm@lists.01.org,
	"Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Oliver O'Halloran <oohall@gmail.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH v10 6/6] powerpc/papr_scm: Implement support for PAPR_PDSM_HEALTH
Date: Sat, 6 Jun 2020 13:09:10 -0500	[thread overview]
Message-ID: <20200606180910.GW31009@gate.crashing.org> (raw)
In-Reply-To: <87wo4kfk58.fsf@linux.ibm.com>

On Sat, Jun 06, 2020 at 06:04:11PM +0530, Vaibhav Jain wrote:
> >> +	/* update health struct with various flags derived from health bitmap */
> >> +	health = (struct nd_papr_pdsm_health) {
> >> +		.dimm_unarmed = p->health_bitmap & PAPR_PMEM_UNARMED_MASK,
> >> +		.dimm_bad_shutdown = p->health_bitmap & PAPR_PMEM_BAD_SHUTDOWN_MASK,
> >> +		.dimm_bad_restore = p->health_bitmap & PAPR_PMEM_BAD_RESTORE_MASK,
> >> +		.dimm_encrypted = p->health_bitmap & PAPR_PMEM_ENCRYPTED,
> >> +		.dimm_locked = p->health_bitmap & PAPR_PMEM_SCRUBBED_AND_LOCKED,
> >> +		.dimm_scrubbed = p->health_bitmap & PAPR_PMEM_SCRUBBED_AND_LOCKED,
> >
> > Are you sure these work?  These are not assignments to a bool so I don't think
> > gcc will do what you want here.
> Yeah, somehow this slipped by and didnt show up in my tests. I checked
> the assembly dump and seems GCC was silently skipping initializing these
> fields without making any noise.

It's not "skipping" that, it initialises the field to 0, just like your
code said it should :-)

If you think GCC should warn for this, please open a PR?  It is *normal*
for bit-fields to be truncated from what is assigned to it, but maybe we
could warn for it in the 1-bit case (we currently don't seem to, even
when the bit-field type is _Bool).

Thanks,


Segher

  reply	other threads:[~2020-06-06 18:10 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04 23:41 [PATCH v10 0/6] powerpc/papr_scm: Add support for reporting nvdimm health Vaibhav Jain
2020-06-04 23:41 ` Vaibhav Jain
2020-06-04 23:41 ` Vaibhav Jain
2020-06-04 23:41 ` [PATCH v10 1/6] powerpc: Document details on H_SCM_HEALTH hcall Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41 ` [PATCH v10 2/6] seq_buf: Export seq_buf_printf Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41 ` [PATCH v10 3/6] powerpc/papr_scm: Fetch nvdimm health information from PHYP Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41 ` [PATCH v10 4/6] powerpc/papr_scm: Improve error logging and handling papr_scm_ndctl() Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-05 17:13   ` Ira Weiny
2020-06-05 17:13     ` Ira Weiny
2020-06-05 17:13     ` Ira Weiny
2020-06-05 19:49     ` Dan Williams
2020-06-05 19:49       ` Dan Williams
2020-06-05 19:49       ` Dan Williams
2020-06-06 11:21       ` Vaibhav Jain
2020-06-06 11:21         ` Vaibhav Jain
2020-06-06 11:21         ` Vaibhav Jain
2020-06-04 23:41 ` [PATCH v10 5/6] ndctl/papr_scm,uapi: Add support for PAPR nvdimm specific methods Vaibhav Jain
2020-06-04 23:41   ` [PATCH v10 5/6] ndctl/papr_scm, uapi: " Vaibhav Jain
2020-06-04 23:41   ` [PATCH v10 5/6] ndctl/papr_scm,uapi: " Vaibhav Jain
2020-06-05 19:49   ` Ira Weiny
2020-06-05 19:49     ` [PATCH v10 5/6] ndctl/papr_scm, uapi: " Ira Weiny
2020-06-05 19:49     ` [PATCH v10 5/6] ndctl/papr_scm,uapi: " Ira Weiny
2020-06-05 20:58     ` Dan Williams
2020-06-05 20:58       ` [PATCH v10 5/6] ndctl/papr_scm, uapi: " Dan Williams
2020-06-05 20:58       ` [PATCH v10 5/6] ndctl/papr_scm,uapi: " Dan Williams
2020-06-04 23:41 ` [PATCH v10 6/6] powerpc/papr_scm: Implement support for PAPR_PDSM_HEALTH Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-04 23:41   ` Vaibhav Jain
2020-06-05 18:36   ` Ira Weiny
2020-06-05 18:36     ` Ira Weiny
2020-06-05 18:36     ` Ira Weiny
2020-06-06 12:34     ` Vaibhav Jain
2020-06-06 12:34       ` Vaibhav Jain
2020-06-06 12:34       ` Vaibhav Jain
2020-06-06 18:09       ` Segher Boessenkool [this message]
2020-06-06 18:09         ` Segher Boessenkool

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=20200606180910.GW31009@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=dan.j.williams@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=oohall@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=santosh@fossix.org \
    --cc=vaibhav@linux.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.