Hello Dan Williams,
The patch 868f036fee4b: "libnvdimm: fix mishandled
nvdimm_clear_poison() return value" from Dec 16, 2016, leads to the
following Smatch static checker warnings:
drivers/nvdimm/claim.c:287 nsio_rw_bytes() warn:
replace divide condition 'cleared / 512' with 'cleared >= 512'
drivers/nvdimm/bus.c:210 nvdimm_account_cleared_poison() warn:
replace divide condition 'cleared / 512' with 'cleared >= 512'
drivers/nvdimm/claim.c
252 static int nsio_rw_bytes(struct nd_namespace_common *ndns,
253 resource_size_t offset, void *buf, size_t size, int rw,
254 unsigned long flags)
255 {
256 struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
257 unsigned int sz_align = ALIGN(size + (offset & (512 - 1)), 512);
258 sector_t sector = offset >> 9;
259 int rc = 0, ret = 0;
260
261 if (unlikely(!size))
262 return 0;
263
264 if (unlikely(offset + size > nsio->size)) {
265 dev_WARN_ONCE(&ndns->dev, 1, "request out of range\n");
266 return -EFAULT;
267 }
268
269 if (rw == READ) {
270 if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align)))
271 return -EIO;
272 if (copy_mc_to_kernel(buf, nsio->addr + offset, size)
!= 0)
273 return -EIO;
274 return 0;
275 }
276
277 if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) {
278 if (IS_ALIGNED(offset, 512) && IS_ALIGNED(size, 512)
279 && !(flags & NVDIMM_IO_ATOMIC)) {
280 long cleared;
281
282 might_sleep();
283 cleared = nvdimm_clear_poison(&ndns->dev,
284 nsio->res.start + offset, size);
285 if (cleared < size)
286 rc = -EIO;
--> 287 if (cleared > 0 && cleared / 512) {
^^^^^^^^^^^^^
Smatch suggests changing this to "&& cleared >= 512" but it doesn't make
sense to say if (cleared > 0 && cleared >= 512) {. Probably what was
instead intended was "if (cleared > 0 && (cleared % 512) == 0) {"?
288 cleared /= 512;
289 badblocks_clear(&nsio->bb, sector,
cleared);
290 }
291 arch_invalidate_pmem(nsio->addr + offset, size);
292 } else
293 rc = -EIO;
294 }
295
296 memcpy_flushcache(nsio->addr + offset, buf, size);
297 ret = nvdimm_flush(to_nd_region(ndns->dev.parent), NULL);
298 if (ret)
299 rc = ret;
300
301 return rc;
302 }
regards,
dan carpenter