On Sat, Oct 31, 2015 at 04:09:56PM +0800, Xiao Guangrong wrote:
> On 10/30/2015 11:54 PM, Eduardo Habkost wrote:
> >On Fri, Oct 30, 2015 at 01:56:01PM +0800, Xiao Guangrong wrote:
> >>There are three places use the some logic to get the page size on
> >>the file path or file fd
> >>
> >>This patch introduces qemu_file_get_page_size() to unify the code
> >>
> >>Signed-off-by: Xiao Guangrong <guangrong.x...@linux.intel.com>
[...]
> >>diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> >>index ac70f08..c661f1c 100644
> >>--- a/target-ppc/kvm.c
> >>+++ b/target-ppc/kvm.c
> >>@@ -308,28 +308,13 @@ static void kvm_get_smmu_info(PowerPCCPU *cpu, struct
> >>kvm_ppc_smmu_info *info)
> >>
> >> static long gethugepagesize(const char *mem_path)
> >> {
> >>- struct statfs fs;
> >>- int ret;
> >>-
> >>- do {
> >>- ret = statfs(mem_path, &fs);
> >>- } while (ret != 0 && errno == EINTR);
> >>+ long size = qemu_file_get_page_size(mem_path);
> >>
> >>- if (ret != 0) {
> >>- fprintf(stderr, "Couldn't statfs() memory path: %s\n",
> >>- strerror(errno));
> >>+ if (!size) {
> >> exit(1);
> >> }
> >>
> >>-#define HUGETLBFS_MAGIC 0x958458f6
> >>-
> >>- if (fs.f_type != HUGETLBFS_MAGIC) {
> >>- /* Explicit mempath, but it's ordinary pages */
> >>- return getpagesize();
> >>- }
> >>-
> >>- /* It's hugepage, return the huge page size */
> >>- return fs.f_bsize;
> >>+ return size;
> >> }
> >
> >Why are you changing target-ppc/kvm.c:gethugepagesize() to use the new
> >funtion, but not the copy at exec.c? To make it simpler, we could
> >eliminate both gethugepagesize() functions completely and replace them
> >with qemu_file_get_page_size() calls (maybe as part of this patch, maybe
> >in a separate patch, I'm not sure).
> >
>
> The gethugepagesize() in exec.c will be eliminated in later patch :).
That's true. I was just expecting to see the change that eliminates
gethugepagesize() in exec.c here instead of patch 08/33. Simple cleanups
that just eliminate code duplication without change in behavior are
easier to review and more likely to be included before the rest of the
series.
>
> And the gethugepagesize() in ppc platform has error handling logic
> and has multiple caller. It's not so bad to keep it.
Well, in case there's still room for cleanup in the ppc code, it may be
done later. No problem.
--
Eduardo
